Re: [PATCH v9 05/13] xen: Add vmware_port support

[Don Slutz <dslutz@verizon.com>]

On 02/17/15 05:30, Andrew Cooper wrote:
> On 16/02/15 23:05, Don Slutz wrote:
>> This includes adding is_vmware_port_enabled

...

>> @@ -5859,6 +5863,12 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
>>                      break;
>>                  if ( a.value > 1 )
>>                      rc = -EINVAL;
>> +                /* Prevent nestedhvm with vmport */
>> +                if ( d->arch.hvm_domain.is_vmware_port_enabled )
>> +                {
>> +                    rc = -EINVAL;
> 
> Probably better as EOPNOTSUPP, as it is a configuration problem.
> 

Will change.

>> +                    break;
>> +                }

>> +int vmport_ioport(int dir, uint32_t port, uint32_t bytes, uint32_t *val)
> 
> This function looks as if it should be static.
> 

Yes, will fix.

>> +{

>> +        case BDOOR_CMD_GETHZ:
>> +        {
>> +            struct segment_register sreg;
>> +
>> +            hvm_get_segment_register(curr, x86_seg_ss, &sreg);
>> +            if ( sreg.attr.fields.dpl == 0 )
>> +            {
> 
> Why is GETHZ the only one of these with a CPL check?

Testing on an ESX server, this is the only one that checks.

Looking at (which also contains backdoor_def.h):

https://github.com/vmware/open-vm-tools.git

It lists some that check for CPL 0:


don-760:~/gits/open-vm-tools>grep "CPL0"
open-vm-tools/lib/include/backdoor_def.h
#define   BDOOR_CMD_APMFUNCTION               2 /* CPL0 only. */
#define   BDOOR_CMD_OSNOTFOUND               18 /* CPL0 only. */
don-760:~/gits/open-vm-tools>grep "CPL 0"
open-vm-tools/lib/include/backdoor_def.h
#define   BDOOR_CMD_INITPCIOPROM             28 /* CPL 0 only. */
#define   BDOOR_CMD_PATCH_SMBIOS_STRUCTS     37 /* CPL 0 only. */
#define   BDOOR_CMD_PATCH_ACPI_TABLES        43 /* CPL 0 only. */
#define   BDOOR_CMD_CHECKFORCEBIOSSETUP      48 /* CPL 0 only. */
#define   BDOOR_CMD_LAZYTIMEREMULATION       49 /* CPL 0 only. */
#define   BDOOR_CMD_BIOSBBS                  50 /* CPL 0 only. */
#define   BDOOR_CMD_OSNOTMACOSXSERVER        54 /* CPL 0 only. */
#define   BDOOR_CMD_XPMODE                   62 /* CPL 0 only. */
#define   BDOOR_CMD_FIRMWARE_INIT            64 /* CPL 0 only. */
#define   BDOOR_CMD_FIRMWARE_ACPI_SERVICES   65 /* CPL 0 only. */
#define   BDOOR_CMD_EFI_SERIALCON_CONFIG     69 /* CPL 0 only. */
#define   BDOOR_CMD_BUG328986                70 /* CPL 0 only. */
#define   BDOOR_CMD_FIRMWARE_ERROR           71 /* CPL 0 only. */
#define   BDOOR_CMD_EFI_BOOT_CONFIG          73 /* CPL 0 only. */
#define   BDOOR_CMD_GET_HW_MODEL             74 /* CPL 0 only. */
#define   BDOOR_CMD_GET_SVGA_CAPABILITIES    75 /* CPL 0 only. */
#define   BDOOR_CMD_GET_FORCE_X2APIC         76 /* CPL 0 only  */
#define   BDOOR_CMD_SET_PCI_HOLE             77 /* CPL 0 only  */
#define   BDOOR_CMD_GET_PCI_HOLE             78 /* CPL 0 only  */
#define   BDOOR_CMD_GET_PCI_BAR              79 /* CPL 0 only  */
#define   BDOOR_CMD_SHOULD_GENERATE_SYSTEMID 80 /* CPL 0 only  */

but not this one:


don-760:~/gits/open-vm-tools>git grep BDOOR_CMD_GETHZ|cat
open-vm-tools/lib/include/backdoor_def.h:#define   BDOOR_CMD_GETHZ
              45

The file I am using in patch #4 is older, and is missing these
comments.

>From https://sites.google.com/site/chitchatvmback/backdoor#cmd12h
...

(*) On WS4.5/GSX3.2 and later this command works only when the virtual
processor is in privileged mode, such as is real mode DOS and OS kernel.
The command is actually implemented also in WS4.0 but is restricted to
work only in the context of virtual machine BIOS and practically
unavailable in the guest OS. On WS3.x/GSX2.5 it works without restriction.

So some of the comments are right.


>> +                value = d->arch.tsc_khz * 1000;

>> diff --git a/xen/include/asm-x86/hvm/vmport.h b/xen/include/asm-x86/hvm/vmport.h
>> new file mode 100644
>> index 0000000..eb3e472
>> --- /dev/null
>> +++ b/xen/include/asm-x86/hvm/vmport.h
>> @@ -0,0 +1,32 @@
>> +/*
>> + * asm/hvm/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. <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#ifndef ASM_X86_HVM_VMPORT_H__
>> +#define ASM_X86_HVM_VMPORT_H__
>> +
>> +void vmport_register(struct domain *d);
> 
> I would suggest putting this declaration in hvm.h and forging vmport.h
> entirely
> 

Ok,

       -Don Slutz

> ~Andrew
> 
>> +int vmport_ioport(int dir, uint32_t port, uint32_t bytes, uint32_t *val);
>> +
>> +#endif /* ASM_X86_HVM_VMPORT_H__ */
>> +
>> +/*
>> + * Local variables:
>> + * mode: C
>> + * c-file-style: "BSD"
>> + * c-basic-offset: 4
>> + * indent-tabs-mode: nil
>> + * End:
>> + */
>> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
>> index b3413a2..28fa767 100644
>> --- a/xen/include/public/domctl.h
>> +++ b/xen/include/public/domctl.h
>> @@ -63,6 +63,9 @@ struct xen_domctl_createdomain {
>>   /* Is this a PVH guest (as opposed to an HVM or PV guest)? */
>>  #define _XEN_DOMCTL_CDF_pvh_guest     4
>>  #define XEN_DOMCTL_CDF_pvh_guest      (1U<<_XEN_DOMCTL_CDF_pvh_guest)
>> + /* Is VMware backdoor port available? */
>> +#define _XEN_DOMCTL_CDF_vmware_port   5
>> +#define XEN_DOMCTL_CDF_vmware_port    (1U<<_XEN_DOMCTL_CDF_vmware_port)
>>      uint32_t flags;
>>  };
>>  typedef struct xen_domctl_createdomain xen_domctl_createdomain_t;
>> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
>> index ccd7ed8..48bb001 100644
>> --- a/xen/include/xen/sched.h
>> +++ b/xen/include/xen/sched.h
>> @@ -546,6 +546,9 @@ struct domain *domain_create(
>>   /* DOMCRF_pvh: Create PV domain in HVM container. */
>>  #define _DOMCRF_pvh             5
>>  #define DOMCRF_pvh              (1U<<_DOMCRF_pvh)
>> + /* DOMCRF_vmware_port: Enable use of vmware backdoor port. */
>> +#define _DOMCRF_vmware_port     6
>> +#define DOMCRF_vmware_port      (1U<<_DOMCRF_vmware_port)
>>  
>>  /*
>>   * rcu_lock_domain_by_id() is more efficient than get_domain_by_id().
> 
>