>>> On 17.02.15 at 00:05, <dsl...@verizon.com> wrote: > This includes adding is_vmware_port_enabled > > This is a new domain_create() flag, DOMCRF_vmware_port. It is > passed to domctl as XEN_DOMCTL_CDF_vmware_port.
As indicated before, I don't think this is a good use case for a domain creation flag. Some of the others we have may not be either, but as I have said quite often recently - let's not make the situation worse by following bad examples. > --- /dev/null > +++ b/xen/arch/x86/hvm/vmware/vmport.c > @@ -0,0 +1,142 @@ > +/* > + * 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. <http://www.gnu.org/licenses/>. > + */ > + > +#include <xen/lib.h> > +#include <asm/hvm/hvm.h> > +#include <asm/hvm/support.h> > +#include <asm/hvm/vmport.h> > + > +#include "backdoor_def.h" > + > +void vmport_register(struct domain *d) > +{ > + register_portio_handler(d, BDOOR_PORT, 4, vmport_ioport); > +} > + > +int vmport_ioport(int dir, uint32_t port, uint32_t bytes, uint32_t *val) static > +{ > + struct cpu_user_regs *regs = guest_cpu_user_regs(); > + > + /* > + * While VMware expects only 32-bit in, they do support using > + * other sizes and out. However they do require only the 1 port > + * and the correct value in eax. Since some of the data > + * returned in eax is smaller the 32 bits and/or you only need > + * the other registers the dir and bytes do not need any > + * checking. The caller will handle the bytes, and dir is > + * handled below. > + */ > + if ( port == BDOOR_PORT && regs->_eax == BDOOR_MAGIC ) > + { > + uint32_t new_eax = BDOOR_MAGIC; > + uint64_t value; > + struct vcpu *curr = current; > + struct domain *d = curr->domain; currd > + > + switch ( regs->_ecx & 0xffff ) > + { > + case BDOOR_CMD_GETMHZ: > + new_eax = d->arch.tsc_khz / 1000; > + break; > + case BDOOR_CMD_GETVERSION: > + /* MAGIC */ > + regs->_ebx = BDOOR_MAGIC; > + /* VERSION_MAGIC */ > + new_eax = 6; > + /* Claim we are an ESX. VMX_TYPE_SCALABLE_SERVER */ > + regs->_ecx = 2; Are you sure you don't want to zero the high halves of 64-bit registers here? > + break; > + case BDOOR_CMD_GETSCREENSIZE: > + /* We have no screen size */ > + new_eax = ~0u; Then just have this handled into the default case. > + break; > + case BDOOR_CMD_GETHWVERSION: > + /* vmware_hw */ > + ASSERT(is_hvm_vcpu(curr)); is_hvm_domain(currd) And - why here rather than before the switch() or even right at the start of the function? > + { > + struct hvm_domain *hd = &d->arch.hvm_domain; > + > + new_eax = hd->params[HVM_PARAM_VMWARE_HWVER]; > + } > + /* > + * Returning zero is not the best. VMware was not at > + * all consistent in the handling of this command until > + * VMware hardware version 4. So it is better to claim > + * 4 then 0. This should only happen in strange configs. > + */ > + if ( !new_eax ) > + new_eax = 4; > + break; > + case BDOOR_CMD_GETHZ: > + { > + struct segment_register sreg; > + > + hvm_get_segment_register(curr, x86_seg_ss, &sreg); > + if ( sreg.attr.fields.dpl == 0 ) > + { > + value = d->arch.tsc_khz * 1000; > + /* apic-frequency (bus speed) */ > + regs->_ecx = 1000000000ULL / APIC_BUS_CYCLE_NS; > + /* High part of tsc-frequency */ > + regs->_ebx = value >> 32; > + /* Low part of tsc-frequency */ > + new_eax = value; > + } > + break; > + } > + case BDOOR_CMD_GETTIME: > + value = get_localtime_us(d) - d->time_offset_seconds * > 1000000ULL; > + /* hostUsecs */ > + regs->_ebx = value % 1000000UL; > + /* hostSecs */ > + new_eax = value / 1000000ULL; > + /* maxTimeLag */ > + regs->_ecx = 1000000; > + /* offset to GMT in minutes */ > + regs->_edx = d->time_offset_seconds / 60; > + break; > + case BDOOR_CMD_GETTIMEFULL: > + value = get_localtime_us(d) - d->time_offset_seconds * > 1000000ULL; > + /* hostUsecs */ > + regs->_ebx = value % 1000000UL; > + /* hostSecs low 32 bits */ > + regs->_edx = value / 1000000ULL; > + /* hostSecs high 32 bits */ > + regs->_esi = (value / 1000000ULL) >> 32; > + /* maxTimeLag */ > + regs->_ecx = 1000000; > + break; No setting of new_eax? > + default: > + new_eax = ~0u; Why is this not the initializer value for the variable then? > + break; > + } > + if ( dir == IOREQ_READ ) > + *val = new_eax; With that, is it really correct that OUT updates the other registers just like IN? If so, this deserves a comment, so that readers won't think this is in error. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel