From mboxrd@z Thu Jan 1 00:00:00 1970 From: Don Slutz Subject: Re: [RFC PATCH 07/10] Add new vmport code. Date: Wed, 18 Dec 2013 21:22:46 -0500 Message-ID: <52B25876.7090409@terremark.com> References: <1386875718-28166-1-git-send-email-dslutz@terremark.com> <1386875718-28166-8-git-send-email-dslutz@terremark.com> <52AA4F83.6020005@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <52AA4F83.6020005@citrix.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: Andrew Cooper Cc: Keir Fraser , Ian Campbell , Stefano Stabellini , Jun Nakajima , Ian Jackson , Eddie Dong , Don Slutz , xen-devel@lists.xen.org, Jan Beulich , Boris Ostrovsky , Suravee Suthikulpanit List-Id: xen-devel@lists.xenproject.org On 12/12/13 19:06, Andrew Cooper wrote: > On 12/12/2013 19:15, Don Slutz wrote: >> From: Don Slutz >> >> enable vmport_flush call. >> >> Signed-off-by: Don Slutz >> --- >> xen/arch/x86/hvm/Makefile | 1 + >> xen/arch/x86/hvm/hvm.c | 2 - >> xen/arch/x86/hvm/vmport/Makefile | 1 + >> xen/arch/x86/hvm/vmport/includeCheck.h | 17 + >> xen/arch/x86/hvm/vmport/vmport.c | 719 +++++++++++++++++++++++++++++++ >> xen/arch/x86/hvm/vmport/xen_vmport_def.h | 36 ++ >> xen/include/asm-x86/hvm/trace.h | 3 + >> 7 files changed, 777 insertions(+), 2 deletions(-) >> create mode 100644 xen/arch/x86/hvm/vmport/Makefile >> create mode 100644 xen/arch/x86/hvm/vmport/includeCheck.h >> create mode 100644 xen/arch/x86/hvm/vmport/vmport.c >> create mode 100644 xen/arch/x86/hvm/vmport/xen_vmport_def.h >> >> diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile >> index eea5555..954a81c 100644 >> --- a/xen/arch/x86/hvm/Makefile >> +++ b/xen/arch/x86/hvm/Makefile >> @@ -1,5 +1,6 @@ >> subdir-y += svm >> subdir-y += vmx >> +subdir-y += vmport >> >> obj-y += asid.o >> obj-y += emulate.o >> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c >> index fa5d382..a557272 100644 >> --- a/xen/arch/x86/hvm/hvm.c >> +++ b/xen/arch/x86/hvm/hvm.c >> @@ -614,9 +614,7 @@ int hvm_domain_initialise(struct domain *d) >> (d->max_pages + (1 << 10) - 1) >> 10, >> (d->max_pages + (1 << 20) - 1) >> 20); >> >> -#if 0 >> vmport_flush(&d->arch.hvm_domain); >> -#endif >> >> if ( is_pvh_domain(d) ) >> { >> diff --git a/xen/arch/x86/hvm/vmport/Makefile b/xen/arch/x86/hvm/vmport/Makefile >> new file mode 100644 >> index 0000000..2648fae >> --- /dev/null >> +++ b/xen/arch/x86/hvm/vmport/Makefile >> @@ -0,0 +1 @@ >> +obj-y += vmport.o >> diff --git a/xen/arch/x86/hvm/vmport/includeCheck.h b/xen/arch/x86/hvm/vmport/includeCheck.h >> new file mode 100644 >> index 0000000..26e0d59 >> --- /dev/null >> +++ b/xen/arch/x86/hvm/vmport/includeCheck.h >> @@ -0,0 +1,17 @@ >> +/* >> + * includeCheck.h >> + * >> + * 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. . >> + */ >> +/* >> + * Nothing here. Just to use backdoor_def.h without change. > What do you mean by this? In VMware it looks like this file that is included by backdoor_def.h does a lot of defines that I have no use for. So to be able to use backdoor_def.h without any changes I needed an include file with the name includeCheck.h; Not sure which is the better way to go. > >> + */ >> diff --git a/xen/arch/x86/hvm/vmport/vmport.c b/xen/arch/x86/hvm/vmport/vmport.c >> new file mode 100644 >> index 0000000..43bdf7b >> --- /dev/null >> +++ b/xen/arch/x86/hvm/vmport/vmport.c >> @@ -0,0 +1,719 @@ >> +/* >> + * 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. . >> + */ >> + >> +#include "xen_vmport_def.h" >> +#include "backdoor_def.h" >> +#include "guest_msg_def.h" >> +#include "asm-x86/hvm/support.h" > #include Will fix. Thanks. > >> + >> +#define LOG_RPC 0x0000001 >> +#define LOG_RECV_STATUS 0x0000002 >> +#define LOG_SKIP_SEND 0x0000004 >> +#define LOG_SEND 0x0000008 >> +#define LOG_SEND_SIZE_ALL 0x0000010 >> +#define LOG_SEND_SIZE 0x0000020 >> +#define LOG_RECV_SIZE_ALL 0x0000040 >> +#define LOG_RECV_SIZE 0x0000080 >> +#define LOG_CLOSE 0x0000100 >> +#define LOG_OPEN 0x0000200 >> +#define LOG_FLUSH 0x0000400 >> +#define LOG_TRACE 0x0000800 >> +#define LOG_PING 0x0001000 >> +#define LOG_SWEEP 0x0002000 >> +#define LOG_BUILD 0x0004000 >> +#define LOG_STATUS 0x0008000 >> + >> +#define LOG_ERROR 0x0010000 >> + >> +#define LOG_INFO_GET 0x0020000 >> +#define LOG_INFO_SET 0x0040000 >> + >> +#define LOG_GP_UNKNOWN 0x0100000 >> +#define LOG_GP_NOT_VMWARE 0x0200000 >> +#define LOG_GP_FAIL_RD_INST 0x0400000 >> +#define LOG_GP_VMWARE_AFTER 0x0800000 >> + >> +#define LOG_VGP_UNKNOWN 0x1000000 >> +#define LOG_REALMODE_GP 0x8000000 >> + >> +extern unsigned long get_sec(void); > Please include the appropriate header files. Will do (assuming you mean for get_sec). >> + >> +/* Note: VMPORT_PORT and VMPORT_MAGIC is also defined as BDOOR_PORT >> + * and BDOOR_MAGIC in backdoor_def.h Defined in vmport.h also. >> + */ >> + >> +inline uint16_t getLowBits(uint32_t bits) >> +{ >> + return bits & 0xffff; >> +} >> + >> +inline uint16_t getHighBits(uint32_t bits) >> +{ >> + return bits >> 16; >> +} >> + >> +inline uint32_t setHighBits(uint32_t b, uint32_t val) >> +{ >> + return (val << 16) | getLowBits(b); >> +} > What is the point of these functions being separated out? It was attempt to may the code more readable. Just noticed that these are in camelCase (getHighBits vs get_high_bits). Which way should I go? > setHighBits only seems to be used with regs->ecx (which is actually a > 64bit value not a 32bit value) to set a status value. > Why not set_status(cpu_user_regs *, uint16_t val) ? Will change to this. > >> + >> +static inline long getLogMask(struct hvm_domain *hd) >> +{ >> + return hd->params[HVM_PARAM_VMPORT_LOGMASK]; > A param is unsigned long, not long... Will fix. >> +} >> + >> +static inline char *getStatus(struct hvm_domain *hd) >> +{ >> + return (char*)&hd->params[HVM_PARAM_VMPORT_STATUS]; > ... And most certainly not a string. Current planning on dropping all code that uses this. >> +} >> + >> +void vmport_safe_print(char *prefix, int len, char *msg) >> +{ >> + unsigned char c; >> + int end = len; >> + int i,k; >> + char out[4*(VMPORT_MAX_SEND_BUF + 1)*3 + 6]; >> + >> + if (end > (sizeof(out)/3 - 6)) >> + end = sizeof(out)/3 - 6; >> + out[0] = '<'; >> + k = 1; >> + for (i = 0; i < end; i++) { >> + c = msg[i]; >> + if ((c == '^') || (c == '\\') || (c == '>')) { >> + out[k++] = '\\'; >> + out[k++] = c; >> + } else if ((c >= ' ') && (c <= '~')) { >> + out[k++] = c; >> + } else if (c < ' ') { >> + out[k++] = '^'; >> + out[k++] = c ^ 0x40; >> + } else { >> + snprintf(&out[k], sizeof(out) - k, "\\%02x", c); >> + k += 3; >> + } >> + } >> + out[k++] = '>'; >> + if (len > end) { >> + out[k++] = '.'; >> + out[k++] = '.'; >> + out[k++] = '.'; >> + } >> + out[k++] = 0; >> + gdprintk(XENLOG_DEBUG, "%s%d(%d,%d,%ld)%s\n", prefix, end, len, k, sizeof(out), out); > the correct format for any sizeof is %zu or %zx depending on decimal/hex. Will fix. >> +} >> + >> +void vmport_send(struct hvm_domain *hd, vmport_channel_t *c, char *msg, int slot) >> +{ >> + unsigned int cur_recv_len = strlen(msg) + 1; >> + char prefix[30]; >> + unsigned int my_bkt = c->recv_write; >> + unsigned int next_bkt = my_bkt + 1; >> + vmport_bucket_t *b; >> + >> + if (next_bkt >= VMPORT_MAX_BKTS) >> + next_bkt = 0; >> + >> + if (next_bkt == c->recv_read) { >> + if (getLogMask(hd) & LOG_SKIP_SEND) { >> + snprintf(prefix, sizeof(prefix), >> + "VMware _send skipped %d (%d, %d) ", c->chan_id, my_bkt, c->recv_read); >> + prefix[sizeof(prefix)-1] = 0; >> + vmport_safe_print(prefix, cur_recv_len, msg); >> + } >> + getStatus(hd)[slot] = 200; > This (and every use of getStatus() below) most certainly needs to > guarentee that slot is strictly in the bound 0 to 7, and writing the > integer 200 to a char is undefined I believe. You are right. Current planning on dropping all code that uses getStatus. > >> + if (getLogMask(hd) & LOG_STATUS) >> + gdprintk(XENLOG_DEBUG, "VMware %d getStatus[%d]=200\n", c->chan_id, slot); >> + return; >> + } >> + >> + c->recv_write = next_bkt; >> + b = &c->recv_bkt[my_bkt]; >> + if (getLogMask(hd) & LOG_SEND) { >> + snprintf(prefix, sizeof(prefix), >> + "VMware _send %d (%d) ", c->chan_id, my_bkt); >> + prefix[sizeof(prefix)-1] = 0; >> + vmport_safe_print(prefix, cur_recv_len, msg); >> + } >> + >> + b->recv_len = cur_recv_len; >> + b->recv_slot = slot; >> + b->recv_idx = 0; >> + memset(b->recv_buf, 0, sizeof(b->recv_buf)); >> + if (cur_recv_len >= (sizeof(b->recv_buf) - 1)) { >> + if (getLogMask(hd) & LOG_ERROR) >> + gdprintk(XENLOG_DEBUG, "VMware recv_len=%d >= %ld.\n", >> + cur_recv_len, sizeof(b->recv_buf) - 1); >> + cur_recv_len = sizeof(b->recv_buf) - 1; >> + } >> + memcpy(b->recv_buf, msg, cur_recv_len); >> + getStatus(hd)[b->recv_slot] = 1; >> + if (getLogMask(hd) & LOG_STATUS) >> + gdprintk(XENLOG_DEBUG, "VMware %d,%d getStatus[%d]=1\n", >> + c->chan_id, c->recv_read, b->recv_slot); >> +} >> + >> +void vmport_ctrl_send(struct hvm_domain *hd, char *msg, int slot) >> +{ >> + struct vmport_state *vs = hd->vmport_data; >> + int i; >> + >> + if (slot < 1 || slot > 7) >> + slot = 7; >> + hd->vmport_data->ping_time = get_sec(); >> + spin_lock(&hd->vmport_lock); >> + for (i = 0; i < VMPORT_MAX_CHANS; i++) { >> + if (vs->chans[i].proto_num == 0x4f4c4354) { >> + vmport_send(hd, &vs->chans[i], msg, slot); >> + } >> + } >> + spin_unlock(&hd->vmport_lock); >> +} >> + >> +void vmport_flush(struct hvm_domain *hd) >> +{ >> + if (getLogMask(hd) & LOG_FLUSH) >> + gdprintk(XENLOG_DEBUG, "VMware flush.\n"); >> + spin_lock(&hd->vmport_lock); >> + memset(&hd->vmport_data->chans, 0, sizeof(hd->vmport_data->chans)); >> + spin_unlock(&hd->vmport_lock); >> +} >> + >> +void vmport_sweep(struct hvm_domain *hd, unsigned long now_time) >> +{ >> + struct vmport_state *vs = hd->vmport_data; >> + int i; >> + >> + for (i = 0; i < VMPORT_MAX_CHANS; i++) { >> + if (vs->chans[i].proto_num) { >> + vmport_channel_t *c = &vs->chans[i]; >> + long delta = now_time - c->active_time; >> + >> + if ( delta >= 80 ) { >> + if (getLogMask(hd) & LOG_SWEEP) >> + gdprintk(XENLOG_DEBUG, "VMware flush %d. delta=%ld\n", >> + c->chan_id, delta); >> + // Return channel to free pool >> + c->proto_num = 0; >> + } >> + } >> + } >> +} >> + >> +vmport_channel_t *vmport_new_chan(struct vmport_state *vs, unsigned long now_time) >> +{ >> + int i; >> + >> + for (i = 0; i < VMPORT_MAX_CHANS; i++) { >> + if (!vs->chans[i].proto_num) { >> + vmport_channel_t *c = &vs->chans[i]; >> + >> + c->chan_id = i; >> + c->cookie = vs->open_cookie++; >> + c->active_time = now_time; >> + c->send_len = 0; >> + c->send_idx = 0; >> + c->recv_read = 0; >> + c->recv_write = 0; >> + return c; >> + } >> + } >> + return NULL; >> +} >> + >> +void vmport_process_send_size(struct hvm_domain *hd, vmport_channel_t *c, struct cpu_user_regs *ur) >> +{ >> + // vmware tools often send a 0 byte request size. >> + c->send_len = ur->ebx; >> + c->send_idx = 0; >> + ur->ecx = setHighBits(ur->ecx, MESSAGE_STATUS_SUCCESS); >> + if ((getLogMask(hd) & LOG_SEND_SIZE_ALL) || >> + ((getLogMask(hd) & LOG_SEND_SIZE) && (c->send_len))) >> + gdprintk(XENLOG_DEBUG, "VMware SENDSIZE %d is %d.\n", >> + c->chan_id, c->send_len); >> +} >> + >> +void vmport_process_send_payload(struct hvm_domain *hd, vmport_channel_t *c, >> + struct cpu_user_regs *ur, unsigned long now_time) >> +{ >> + char prefix[30]; >> + >> + if (c->send_idx < VMPORT_MAX_SEND_BUF) { >> + c->send_buf[c->send_idx] = ur->ebx; >> + } >> + c->send_idx++; >> + ur->ecx = setHighBits(ur->ecx, MESSAGE_STATUS_SUCCESS); >> + if (c->send_idx * 4 >= c->send_len) { >> + if (c->send_idx < VMPORT_MAX_SEND_BUF) >> + ((char*)c->send_buf)[c->send_len] = 0; >> + if (getLogMask(hd) & LOG_RPC) { >> + snprintf(prefix, sizeof(prefix), >> + "VMware RPC %d (%d) ", c->chan_id, c->recv_read); >> + prefix[sizeof(prefix)-1] = 0; >> + vmport_safe_print(prefix, c->send_len, (char*)c->send_buf); >> + } >> + if (c->proto_num == 0x49435052) { >> +/* log toolbox: Version: build-341836 */ >> +/* SetGuestInfo 4 build-341836 */ >> +/* info-get guestinfo.ip */ >> +/* info-set guestinfo.ip joe */ >> + char * build = NULL; >> + char * info_key = NULL; >> + char * ret_msg = "1 "; > I cant remember offhand whether Xen is compiled with or without > read-only strings, Not sure why it matters. What ret_msg points to is not changed. ret_msg it's self is changed to point to different strings. 0 hvm/vmport/vmport.c vmport_process_send_payload 274 char * ret_msg = "1 "; 1 hvm/vmport/vmport.c vmport_process_send_payload 311 ret_msg = ret_buffer; 2 hvm/vmport/vmport.c vmport_process_send_payload 316 ret_msg = "0 No value found"; 3 hvm/vmport/vmport.c vmport_process_send_payload 319 ret_msg = "0 Key is too long"; 4 hvm/vmport/vmport.c vmport_process_send_payload 364 ret_msg = "0 Too many keys"; 5 hvm/vmport/vmport.c vmport_process_send_payload 373 ret_msg = "0 Value too long"; 6 hvm/vmport/vmport.c vmport_process_send_payload 376 ret_msg = "0 Key is too long"; 7 hvm/vmport/vmport.c vmport_process_send_payload 384 ret_msg = "0 Two and exactly two arguments expected"; 8 hvm/vmport/vmport.c vmport_process_send_payload 388 vmport_send(hd, c, ret_msg, 5); And it looks like I should change: void vmport_send(struct hvm_domain *hd, vmport_channel_t *c, char *msg, int slot) into: void vmport_send(struct hvm_domain *hd, vmport_channel_t *c, const char *msg, int slot) >> + char ret_buffer[2 + VMPORT_MAX_VAL_LEN + 2]; >> + >> + if (strncmp((char*)c->send_buf, "log toolbox: Version: build-", >> + strlen("log toolbox: Version: build-")) == 0) { >> + build = (char*)c->send_buf + strlen("log toolbox: Version: build-"); >> + } else if (strncmp((char*)c->send_buf, "SetGuestInfo 4 build-", >> + strlen("SetGuestInfo 4 build-")) == 0) { >> + build = (char*)c->send_buf + strlen("SetGuestInfo 4 build-"); >> + } else if (strncmp((char*)c->send_buf, "info-get guestinfo.", >> + strlen("info-get guestinfo.")) == 0) { >> + int keyLen = c->send_len - strlen("info-get guestinfo."); >> + int idx; >> + struct vmport_state *vs = hd->vmport_data; >> + >> + info_key = (char*)c->send_buf + strlen("info-get guestinfo."); >> + if (getLogMask(hd) & LOG_INFO_GET) { >> + snprintf(prefix, sizeof(prefix), >> + "VMware info-get key:"); >> + vmport_safe_print(prefix, keyLen, info_key); >> + } >> + if (keyLen <= VMPORT_MAX_KEY_LEN) { >> + for (idx = 0; idx < vs->used_guestinfo; idx++) { >> + if ((vs->guestinfo[idx]->key_len == keyLen) && >> + (memcmp(info_key, >> + vs->guestinfo[idx]->key_data, >> + vs->guestinfo[idx]->key_len) == 0)) { >> + if (getLogMask(hd) & LOG_INFO_GET) { >> + snprintf(prefix, sizeof(prefix), >> + "VMware info-get val:"); >> + vmport_safe_print(prefix, >> + vs->guestinfo[idx]->val_len, >> + vs->guestinfo[idx]->val_data); >> + } >> + snprintf(ret_buffer, sizeof(ret_buffer) - 1, "1 %.*s", >> + (int)vs->guestinfo[idx]->val_len, >> + vs->guestinfo[idx]->val_data); >> + ret_msg = ret_buffer; >> + break; >> + } >> + } >> + if (idx >= vs->used_guestinfo) { >> + ret_msg = "0 No value found"; >> + } >> + } else { >> + ret_msg = "0 Key is too long"; >> + } >> + } else if (strncmp((char*)c->send_buf, "info-set guestinfo.", >> + strlen("info-set guestinfo.")) == 0) { >> + char * val; >> + int rest_len = c->send_len - strlen("info-set guestinfo."); >> + >> + info_key = (char*)c->send_buf + strlen("info-set guestinfo."); >> + val = strstr(info_key, " "); >> + if (val) { >> + int keyLen = val - info_key; >> + int valLen = rest_len - keyLen - 1; >> + int free_idx = -1; >> + int idx; >> + struct vmport_state *vs = hd->vmport_data; >> + >> + val++; >> + if (getLogMask(hd) & LOG_INFO_SET) { >> + snprintf(prefix, sizeof(prefix), >> + "VMware info-set key:"); >> + vmport_safe_print(prefix, keyLen, info_key); >> + snprintf(prefix, sizeof(prefix), >> + "VMware info-set val:"); >> + vmport_safe_print(prefix, valLen, val); >> + } >> + if (keyLen <= VMPORT_MAX_KEY_LEN) { >> + if (valLen <= VMPORT_MAX_VAL_LEN) { >> + 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 == keyLen) && >> + (memcmp(info_key, >> + vs->guestinfo[idx]->key_data, >> + vs->guestinfo[idx]->key_len) == 0)) { >> + vs->guestinfo[idx]->val_len = valLen; >> + memcpy(vs->guestinfo[idx]->val_data, val, valLen); >> + break; >> + } else if ((vs->guestinfo[idx]->key_len == 0) && >> + (free_idx == -1)) { >> + free_idx = idx; >> + } >> + } >> + if (idx >= vs->used_guestinfo) { >> + if (free_idx == -1) { >> + ret_msg = "0 Too many keys"; >> + } else { >> + vs->guestinfo[free_idx]->key_len = keyLen; >> + memcpy(vs->guestinfo[free_idx]->key_data, info_key, keyLen); >> + vs->guestinfo[free_idx]->val_len = valLen; >> + memcpy(vs->guestinfo[free_idx]->val_data, val, valLen); >> + } >> + } >> + } else { >> + ret_msg = "0 Value too long"; >> + } >> + } else { >> + ret_msg = "0 Key is too long"; >> + } >> + } else { >> + if (getLogMask(hd) & LOG_INFO_SET) { >> + snprintf(prefix, sizeof(prefix), >> + "VMware info-set missing val; key:"); >> + vmport_safe_print(prefix, rest_len, info_key); >> + } >> + ret_msg = "0 Two and exactly two arguments expected"; >> + } >> + } >> + >> + vmport_send(hd, c, ret_msg, 5); >> + if (build) { >> + long val = 0; >> + char *p = build; >> + >> + while (*p) { >> + if (*p < '0' || *p > '9') >> + break; >> + val = val * 10 + *p - '0'; >> + p++; >> + }; >> + >> + hd->params[HVM_PARAM_VMPORT_BUILD_NUMBER_VALUE] = val; >> + hd->params[HVM_PARAM_VMPORT_BUILD_NUMBER_TIME] = now_time; >> + if (getLogMask(hd) & LOG_BUILD) { >> + snprintf(prefix, sizeof(prefix), >> + "VMware build %ld ", val); >> + vmport_safe_print(prefix, p - build, build); >> + } >> + } >> + } else { >> + unsigned int my_bkt = c->recv_read - 1; >> + vmport_bucket_t *b; >> + int stat = 100; >> + int slot; >> + >> + if (my_bkt >= VMPORT_MAX_BKTS) >> + my_bkt = VMPORT_MAX_BKTS - 1; >> + b = &c->recv_bkt[my_bkt]; >> + b->recv_len = 0; >> + slot = b->recv_slot; >> + if (slot < 1 || slot > 7) >> + slot = 7; >> + if ((c->send_len > 2) && ((c->send_buf[0] & 0xffff) == 0x4b4f)) >> + stat = 3; >> + if (getLogMask(hd) & LOG_STATUS) >> + gdprintk(XENLOG_DEBUG, "VMware %d,%d(%d) getStatus[%d(%d)]=%d <== %d hex=0x%x\n", >> + c->chan_id, my_bkt, c->recv_read, slot, b->recv_slot, >> + getStatus(hd)[slot], stat, c->send_buf[0] & 0xffff); >> + getStatus(hd)[slot] = stat; >> + } >> + } >> +} >> + >> +void vmport_process_recv_size(struct hvm_domain *hd, vmport_channel_t *c, struct cpu_user_regs *ur) >> +{ >> + vmport_bucket_t *b = &c->recv_bkt[c->recv_read]; >> + >> + if ((getLogMask(hd) & LOG_RECV_SIZE_ALL) || >> + ((getLogMask(hd) & LOG_RECV_SIZE) && (b->recv_len))) >> + gdprintk(XENLOG_DEBUG, "VMware RECVSIZE %d is %d.\n", >> + c->chan_id, b->recv_len); >> + >> + if (b->recv_len) { >> + ur->ecx = setHighBits(ur->ecx, MESSAGE_STATUS_DORECV | MESSAGE_STATUS_SUCCESS); >> + ur->edx = setHighBits(ur->edx, MESSAGE_TYPE_SENDSIZE); >> + ur->ebx = b->recv_len; >> + } else { >> + ur->ecx = setHighBits(ur->ecx, MESSAGE_STATUS_SUCCESS); >> + } >> +} >> + >> +void vmport_process_recv_payload(struct hvm_domain *hd, vmport_channel_t *c, struct cpu_user_regs *ur) >> +{ >> + vmport_bucket_t *b = &c->recv_bkt[c->recv_read]; >> + >> + if (b->recv_idx < VMPORT_MAX_RECV_BUF) { >> + ur->ebx = b->recv_buf[b->recv_idx++]; >> + } else { >> + ur->ebx = 0; >> + } >> + ur->ecx = setHighBits(ur->ecx, MESSAGE_STATUS_SUCCESS); >> + ur->edx = setHighBits(ur->edx, MESSAGE_TYPE_SENDPAYLOAD); >> +} >> + >> +void vmport_process_recv_status(struct hvm_domain *hd, vmport_channel_t *c, struct cpu_user_regs *ur) >> +{ >> + vmport_bucket_t *b = &c->recv_bkt[c->recv_read]; >> + char prefix[30]; >> + >> + ur->ecx = setHighBits(ur->ecx, MESSAGE_STATUS_SUCCESS); >> + if (getLogMask(hd) & LOG_RECV_STATUS) { >> + snprintf(prefix, sizeof(prefix), >> + "VMware RECVSTATUS %d (%d) ", c->chan_id, c->recv_read); >> + prefix[sizeof(prefix)-1] = 0; >> + vmport_safe_print(prefix, b->recv_len, (char*)b->recv_buf); >> + } >> + getStatus(hd)[b->recv_slot] = 2; >> + if (getLogMask(hd) & LOG_STATUS) >> + gdprintk(XENLOG_DEBUG, "VMware %d,%d getStatus[%d]=2\n", >> + c->chan_id, c->recv_read, b->recv_slot); >> + c->recv_read++; >> + if (c->recv_read >= VMPORT_MAX_BKTS) >> + c->recv_read = 0; >> +} >> + >> +void vmport_process_close(struct hvm_domain *hd, vmport_channel_t *c, struct cpu_user_regs *ur) >> +{ >> + // Return channel to free pool >> + c->proto_num = 0; >> + ur->ecx = setHighBits(ur->ecx, MESSAGE_STATUS_SUCCESS); >> + if (getLogMask(hd) & LOG_CLOSE) >> + gdprintk(XENLOG_DEBUG, "VMware CLOSE %d.\n", >> + c->chan_id); >> + if (getLogMask(hd) & LOG_STATUS) >> + gdprintk(XENLOG_DEBUG, "VMware %d getStatus[0]=0x%x & ~0x%x=0x%x\n", >> + c->chan_id, getStatus(hd)[0], 1 << c->chan_id, ~(1 << c->chan_id)); >> + getStatus(hd)[0] &= ~(1 << c->chan_id); >> +} >> + >> +void vmport_process_packet(struct hvm_domain *hd, vmport_channel_t *c, >> + struct cpu_user_regs *ur, int sub_cmd, >> + unsigned long now_time) >> +{ >> + c->active_time = now_time; >> + switch (sub_cmd) { >> + case MESSAGE_TYPE_SENDSIZE: >> + vmport_process_send_size(hd, c, ur); >> + break; >> + case MESSAGE_TYPE_SENDPAYLOAD: >> + vmport_process_send_payload(hd, c, ur, now_time); >> + break; >> + >> + case MESSAGE_TYPE_RECVSIZE: >> + vmport_process_recv_size(hd, c, ur); >> + break; >> + case MESSAGE_TYPE_RECVPAYLOAD: >> + vmport_process_recv_payload(hd, c, ur); >> + break; >> + case MESSAGE_TYPE_RECVSTATUS: >> + vmport_process_recv_status(hd, c, ur); >> + break; >> + >> + case MESSAGE_TYPE_CLOSE: >> + vmport_process_close(hd, c, ur); >> + break; >> + >> + default: >> + ur->ecx = 0; >> + break; >> + } >> +} >> + >> +void vmport_rpc(struct hvm_domain *hd, struct cpu_user_regs *ur) >> +{ >> + int sub_cmd = (ur->ecx >> 16) & 0xffff; >> + vmport_channel_t *c = NULL; >> + uint16_t msg_id; >> + uint32_t msg_cookie; >> + unsigned long now_time = get_sec(); >> + long delta = now_time - hd->vmport_data->ping_time; >> + >> + if ( delta >= hd->params[HVM_PARAM_VMPORT_RESET_TIME] ) { >> + if (getLogMask(hd) & LOG_PING) >> + gdprintk(XENLOG_DEBUG, "VMware ping. delta=%ld\n", >> + delta); >> + vmport_ctrl_send(hd, "reset", 7); >> + } >> + spin_lock(&hd->vmport_lock); >> + vmport_sweep(hd, now_time); >> + do { >> + // Check to see if a new open request is happening... >> + if (MESSAGE_TYPE_OPEN == sub_cmd) { >> + c = vmport_new_chan(hd->vmport_data, now_time); >> + if (NULL == c) { >> + if (getLogMask(hd) & LOG_ERROR) >> + gdprintk(XENLOG_ERR, "VMware failed to find a free channel.\n"); >> + break; >> + } >> + >> + // Attach the apropriate protocol the the channel >> + c->proto_num = ur->ebx & ~GUESTMSG_FLAG_COOKIE; >> + ur->ecx = setHighBits(ur->ecx, MESSAGE_STATUS_SUCCESS); >> + ur->edx = setHighBits(ur->edx, c->chan_id); >> + ur->edi = getLowBits(c->cookie); >> + ur->esi = getHighBits(c->cookie); >> + if (getLogMask(hd) & LOG_OPEN) >> + gdprintk(XENLOG_DEBUG, "VMware OPEN %d p=%x.\n", >> + c->chan_id, c->proto_num); >> + if (getLogMask(hd) & LOG_STATUS) >> + gdprintk(XENLOG_DEBUG, "VMware %d getStatus[0]=0x%x | 0x%x\n", >> + c->chan_id, getStatus(hd)[0], 1 << c->chan_id); >> + getStatus(hd)[0] |= 1 << c->chan_id; >> + if (c->proto_num == 0x4f4c4354) { >> + vmport_send(hd, c, "reset", 6); >> + } >> + break; >> + } >> + >> + msg_id = getHighBits(ur->edx); >> + msg_cookie = getLowBits(ur->edi) | (ur->esi << 16); >> + if (msg_id >= VMPORT_MAX_CHANS) { >> + if (getLogMask(hd) & LOG_ERROR) >> + gdprintk(XENLOG_ERR, "VMware chan id err %d >= %d.\n", >> + msg_id, VMPORT_MAX_CHANS); >> + break; >> + } >> + c = &hd->vmport_data->chans[msg_id]; >> + if (!c->proto_num) { >> + if (getLogMask(hd) & LOG_ERROR) >> + gdprintk(XENLOG_ERR, "VMware chan %d not open.\n", >> + msg_id); >> + break; >> + } >> + >> + // We check the cookie here since it's possible that the >> + // connection timed out on us and another channel was opened >> + // if this happens, return error and the um tool will >> + // need to reopen the connection >> + if (msg_cookie != c->cookie) { >> + if (getLogMask(hd) & LOG_ERROR) >> + gdprintk(XENLOG_ERR, "VMware cookie err %x vs %x.\n", >> + msg_cookie, c->cookie); >> + break; >> + } >> + vmport_process_packet(hd, c, ur, sub_cmd, now_time); >> + } while( 0 ); >> + >> + if( NULL == c ) >> + ur->ecx = setHighBits(ur->ecx, 0); >> + >> + spin_unlock(&hd->vmport_lock); >> +} >> + >> +int vmport_ioport(int dir, int size, unsigned long data, struct cpu_user_regs *regs) >> +{ >> + uint32_t cmd = getLowBits(regs->ecx); >> + uint32_t magic = regs->eax; >> + struct hvm_domain *hd = ¤t->domain->arch.hvm_domain; >> + >> + if ( dir != IOREQ_WRITE ) >> + data = 0; >> + >> + if (magic == BDOOR_MAGIC) { >> + const uint32_t apicHz = 1000000000L; > Again this hard coded constant. Will be looking into how to fix correctly. > >> + uint64_t value; >> + >> + switch (cmd) { >> + case BDOOR_CMD_GETMHZ: >> + /* ... */ >> + regs->ebx = BDOOR_MAGIC; >> + regs->eax = (uint32_t)(current->domain->arch.tsc_khz / 1000); >> + break; >> + case BDOOR_CMD_GETVERSION: >> + /* ... */ >> + regs->ebx = BDOOR_MAGIC; >> + /* VERSION_MAGIC */ >> + regs->eax = 6; >> + /* Claim we are an ESX. VMX_TYPE_SCALABLE_SERVER */ >> + regs->ecx = 2; >> + break; >> + case BDOOR_CMD_GETHWVERSION: >> + /* ... */ >> + regs->ebx = BDOOR_MAGIC; >> + /* ?? */ >> + regs->eax = 0x4; >> + break; >> + case BDOOR_CMD_GETHZ: >> + value = current->domain->arch.tsc_khz * 1000; >> + /* apic-frequency (bus speed) */ >> + regs->ecx = apicHz; >> + /* High part of tsc-frequency */ >> + regs->ebx = (uint32_t)(value >> 32); >> + /* Low part of tsc-frequency */ >> + regs->eax = (uint32_t)value; >> + break; >> + case BDOOR_CMD_GETTIME: >> + value = get_localtime_us(current->domain); >> + /* hostUsecs */ >> + regs->ebx = (uint32_t)(value % 1000000UL); >> + /* hostSecs */ >> + regs->eax = (uint32_t)(value / 1000000ULL); >> + /* maxTimeLag */ >> + regs->ecx = 0; >> + break; >> + case BDOOR_CMD_GETTIMEFULL: >> + value = get_localtime_us(current->domain); >> + /* ... */ >> + regs->eax = BDOOR_MAGIC; >> + /* hostUsecs */ >> + regs->ebx = (uint32_t)(value % 1000000UL); >> + /* High part of hostSecs */ >> + regs->esi = (uint32_t)((value / 1000000ULL) >> 32); >> + /* Low part of hostSecs */ >> + regs->edx = (uint32_t)(value / 1000000ULL); >> + /* maxTimeLag */ >> + regs->ecx = 0; >> + break; >> + case BDOOR_CMD_MESSAGE: >> + vmport_rpc(hd, regs); >> + break; >> + >> + default: >> + if (getLogMask(hd) & LOG_ERROR) >> + gdprintk(XENLOG_DEBUG, "VMware size=%d dir=%d data=%lx cmd=%d.\n", >> + size, dir, data, cmd); >> + break; >> + } >> + if (getLogMask(hd) & LOG_TRACE) >> + gdprintk(XENLOG_DEBUG, "VMware ip=%lx cmd=%d ax=%lx bx=%lx cx=%lx dx=%lx si=%lx di=%lx\n", >> + (unsigned long)regs->eip, cmd, >> + (unsigned long)regs->eax, (unsigned long)regs->ebx, >> + (unsigned long)regs->ecx, (unsigned long)regs->edx, >> + (unsigned long)regs->esi, (unsigned long)regs->edi); >> + } else >> + if (getLogMask(hd) & LOG_ERROR) >> + gdprintk(XENLOG_ERR, "Not VMware %x vs %x vs %x; ip=%lx ax=%lx bx=%lx cx=%lx dx=%lx si=%lx di=%lx\n", >> + magic, BDOOR_MAGIC, VMPORT_MAGIC, >> + (unsigned long)regs->eip, >> + (unsigned long)regs->eax, (unsigned long)regs->ebx, >> + (unsigned long)regs->ecx, (unsigned long)regs->edx, >> + (unsigned long)regs->esi, (unsigned long)regs->edi); >> + >> + if (dir == IOREQ_READ) >> + HVMTRACE_ND(IOPORT_READ, 0, 1/*cycles*/, 5, VMPORT_PORT, cmd, >> + regs->eax, regs->ebx, regs->ecx, 0); >> + else >> + HVMTRACE_ND(IOPORT_WRITE, 0, 1/*cycles*/, 5, VMPORT_PORT, cmd, >> + regs->eax, regs->ebx, regs->ecx, 0); >> + >> + return 1; >> +} >> + >> +/* >> + * Local variables: >> + * mode: C >> + * c-set-style: "BSD" >> + * c-basic-offset: 4 >> + * tab-width: 4 >> + * indent-tabs-mode: nil >> + * End: >> + */ > Frankly, this while file is quite nasty. There is a huge amount of > pointer arithmetic and array indexing with guest data, which doesn't > appear at a glance to have sufficient validation. Will look into make it clearer. > The vast majority of the ints should be unsigned as they are used as > indices. All the times should probably be s_time_t's Will fix. > The idiom > > if ( getLogMask(hd) & LOG_$SOMETHING ) > gdprintk(XENLOG_$SOMETHING, ...); > > Can probably be moved into a single macro, which will simply the reading > of the code quite a bit. Will do. > > Is there a spec for what this code is trying to accomplish? Some of the > comments suggest that it has been reverse engineered? I am not aware of such a spec. I will attempt to find one. It looks like add what I know as a documnet file would be helpful. Some of it has been "reverse" engineered in that it comes from decoding the code provided by: http://packages.vmware.com/tools/esx/3.5latest/rhel4/SRPMS/index.html open-vm-tools-kmod-7.4.8-396269.423167.src.rpm And also seeing the debug output of doing some actions using VMware tools. > > I think I have counted 4 coding styles in there. Given the Xen block on > the bottom, can it settle on a single style. Sure, I will working on getting this to 1 style. > >> diff --git a/xen/arch/x86/hvm/vmport/xen_vmport_def.h b/xen/arch/x86/hvm/vmport/xen_vmport_def.h >> new file mode 100644 >> index 0000000..e87845b >> --- /dev/null >> +++ b/xen/arch/x86/hvm/vmport/xen_vmport_def.h >> @@ -0,0 +1,36 @@ >> +/* >> + * xen_vmport_def.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 __XEN_VMPORT_DEF_H__ >> +#define __XEN_VMPORT_DEF_H__ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include > What is the point of this header file? At some point in the past there was multiple users. I see that there is only one now, and so will be removed. > >> + >> +#endif >> diff --git a/xen/include/asm-x86/hvm/trace.h b/xen/include/asm-x86/hvm/trace.h >> index 9d7e00b..d5c3a3e 100644 >> --- a/xen/include/asm-x86/hvm/trace.h >> +++ b/xen/include/asm-x86/hvm/trace.h >> @@ -52,8 +52,11 @@ >> #define DO_TRC_HVM_LMSW64 DEFAULT_HVM_MISC >> #define DO_TRC_HVM_REALMODE_EMULATE DEFAULT_HVM_MISC >> #define DO_TRC_HVM_TRAP DEFAULT_HVM_MISC >> +#define DO_TRC_HVM_TRAP64 DEFAULT_HVM_MISC >> #define DO_TRC_HVM_TRAP_DEBUG DEFAULT_HVM_MISC >> #define DO_TRC_HVM_VLAPIC DEFAULT_HVM_MISC >> +#define DO_TRC_HVM_IOPORT_READ DEFAULT_HVM_IO >> +#define DO_TRC_HVM_IOPORT_WRITE DEFAULT_HVM_IO > There are already traps for io ports, which I believe are part of the > generic vmexit traps. > Looks like the name is not good. And normal ports to not use or change other cpu registers. These last 2 are used: if (dir == IOREQ_READ) HVMTRACE_ND(IOPORT_READ, 0, 1/*cycles*/, 5, VMPORT_PORT, cmd, regs->eax, regs->ebx, regs->ecx, 0); else HVMTRACE_ND(IOPORT_WRITE, 0, 1/*cycles*/, 5, VMPORT_PORT, cmd, regs->eax, regs->ebx, regs->ecx, 0); I.E. they are VMware backdoor port only traces. As such, it would be good to drop the fixed and duplicated data, and add the missing data. I.E. Change them to: if (dir == IOREQ_READ) HVMTRACE_ND(IOPORT_READ, 0, 1/*cycles*/, 6, regs->eax, regs->ebx, regs->ecx, regs->edx, regs->esi, regs->edi); else HVMTRACE_ND(IOPORT_WRITE, 0, 1/*cycles*/, 6, regs->eax, regs->ebx, regs->ecx, regs->edx, regs->esi, regs->edi); With IOPORT_READ changed also to something like VMWARE_PORT_READ. -Don Slutz > ~Andrew > >> >> >> #define TRC_PAR_LONG(par) ((par)&0xFFFFFFFF),((par)>>32)