All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v10 00/10] Xen VMware tools support
@ 2015-05-14 23:34 Don Slutz
  2015-05-14 23:34 ` [PATCH v10 01/10] tools: Add vga=vmware Don Slutz
                   ` (9 more replies)
  0 siblings, 10 replies; 22+ messages in thread
From: Don Slutz @ 2015-05-14 23:34 UTC (permalink / raw)
  To: xen-devel
  Cc: Kevin Tian, Keir Fraser, Ian Campbell, Stefano Stabellini,
	Jun Nakajima, Eddie Dong, Ian Jackson, Don Slutz, Tim Deegan,
	George Dunlap, Aravind Gopalakrishnan, Jan Beulich,
	Andrew Cooper, Boris Ostrovsky, Suravee Suthikulpanit

Changes v9 to v10:
  Split out LIBXL_VGA_INTERFACE_TYPE_VMWARE into it's own patch (#1)
  that can stand alone.  In the patch set because a later patch
  depends on it.

  Reworked to be based on:

    commit a7511905fae7ba592c5bf63cd77d8ff78087d689
    Author: Julien Grall <julien.grall@linaro.org>
    Date:   Wed Apr 1 17:21:41 2015 +0100

        xen: Extend DOMCTL createdomain to support arch configuration

  rebased onto:

    commit e13013dbf1d5997915548a3b5f1c39594d8c1d7b
    Author: Yang Hongyang <yanghy@cn.fujitsu.com>
    Date:   Thu May 14 16:55:18 2015 +0800

        libxc/restore: add checkpointed flag to the restore context


  Andrew Cooper (#2: "xen: Add support for VMware cpuid leaves"):
    Did not add "Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>"
    because of changes here to do things the new way.
  Reword comment message to reflect new way.

  Ian Campbell (#3 "tools: Add vmware_hwver support"):
    LIBXL_HAVE_LIBXL_VGA_INTERFACE_TYPE_VMWARE &
    LIBXL_HAVE_BUILDINFO_HVM_VMWARE_HWVER are arriving together
    a single umbrella could be used.
      Since I split the LIBXL_VGA_INTERFACE_TYPE_VMWARE into
      it's own patch, this is not longer true.
      But I did use 1 for the 2 c_info changes.
    Please use GCSPRINTF.
      Done.
  Remove vga=vmware from here.

  Ian Campbell (#3 "tools: Add vmware_hwver support"):
    For "Add IOREQ_TYPE_VMWARE_PORT"
      With those fixed the tools/* bits are:
        Acked-by: Ian Campbell <ian.campbell@citrix.com>  
    Did not add Acked-by to "tools: Add vmware_hwver support"
    because of the rework for using libxl_domain_create_info.

  Andrew Cooper (#4: "vmware: Add VMware provided include file."):
    Added "Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>"

  Andrew Cooper (#5 "xen: Add vmware_port support"):
    Probably better as EOPNOTSUPP, as it is a configuration problem.
      Done.
    vmport_ioport function looks as if it should be static.
      Done.
    Why is GETHZ the only one of these with a CPL check?
      Please see thread for detail.
    I would suggest putting vmport_register declaration in hvm.h ...
      Done.

  Jan Beulich (#5 "xen: Add vmware_port support"):
    As indicated before, I don't think this is a good use case for a
    domain creation flag.
      Switch to the new config way.
    struct domain *d => struct domain *currd
      Done
    Are you sure you don't want to zero the high halves of 64-bit ...
      Comment added.
   Then just have this handled into the default case.
      Reworked new_eax handling.
   is_hvm_domain(currd)
   And - why here rather than before the switch() or even right at the
   start of the function?
      Moved to start.
   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.
     All done in comment at start.

  Andrew Cooper (#6 "xen: Add ring 3 vmware_port support"):
    >> This looks horribly invasive.
    >>
    >> Why are emulation changes needed?  What is wrong with the normal
    >> handling with a registered ioport handler?
    > Because VMware made a bad way to provide a "hyper call".  They decided to
    > allow user access to this.  So when a #GP fault should have been
    > reported, they instead do the "hyper call".
    >
    Urgh - now I remember.

    Right.  In the case that vmport is active, we start intercepting #GP
    faults and emulating access.  That part of the patch looks ok.

    However, the rest is very invasive to the emulation infrastructure.
      Re-worked along this lines suggested.

  Jan Beulich (#6 "xen: Add ring 3 vmware_port support"):
    I hope that vmport_check will no longer be needed with the adjustments ...
    > Since this is not an architecture feature and I do not expect any real
    > CPUs to support this, I do not expect any other use.  But I am happy
    > to make it more generic.

    Let's see how this ends up looking - the hook is probably indeed
    bogus (from an architectural pov) no matter how you name it.
      Last e-mail on thread, so no change.

  Ian Campbell (#7 "tools: Add vmware_port support"):
    If..." at the start of the sentence ...
      Used Ian's reword.
    Also, why is 7 special?
      Attempted to better explain.

  Paul Durrant & Jan Beulich (#8 "Add IOREQ_TYPE_VMWARE_PORT"):
    Now that buf is no longer a bool, could ...
    These literals should become an enum
      Added an enum.
    I don't think the invalidate type is needed.
      Dropped.
    IOREQ_TYPE_VMWARE_PORT as 3 is a re-use.
      Switch to 9.
    Code handling "case X86EMUL_UNHANDLEABLE:" in emulate.c
    is unclear.
       Re-worked to a version that Jan likes better.
    Comment about "special' range of 1" is not clear.
       Re-worded comments.

  Ian Campbell (#9 "Add xentrace to vmware_port"):
    Acked-by
  Readded dropped traces.

  Jan Beulich & Andrew Cooper (#9 "Add xentrace to vmware_port"):
    Why is cmd in this patch?
      Because the trace points use it.

  Jan Beulich (#10 "test_x86_emulator.c: Add tests for #GP usage"):
    Need more comments and simpler error checking.
      Done.  
      Dropped un-needed new routines.

  Andrew Cooper:
    That is because you broke it adding a bool_t item.
      Has now been dropped.


Changes v8 to v9:
  Overview of changes:
    s/vmware_hw/vmware_hwver/i
    Switch to x86_emulator to handle #GP
    New patch: Move MAX_INST_LEN into x86_emulate.h
    Add QEMU usage, patch #8 "Add IOREQ_TYPE_VMWARE_PORT"
    Split patch "xen: Add vmware_port support" into 2. 1st has same
    name.  New one is "xen: Add ring 3 vmware_port support".
    Added 3 new patches about test_x86_emulator.

  
  Jan Beulich (#2: "xen: Add support for VMware cpuid leaves"):
    Change -EXDEV to EOPNOTSUPP.
      Done.
    adding another subdirectory: xen/arch/x86/hvm/vmware
    Much will depend on the discussion of the subsequent patches.
      TBD.
    So for versions < 7 there's effectively no CPUID support at all?
      Changed to check at entry.
    The comment /* Params for VMware */ seems wrong...
      Changed to /* emulated VMware Hardware Version */
    Also please use d, not _d in #define is_vmware_domain()
      Changed.  Line is now > 80 characters, so split into 2.

  Andrew Cooper (#3: "tools: Add vmware_hwver support"):
      I assumed that s/vmware_hw/vmware_hwver/ is not a big enough
      change to drop the Reviewed-by.  Did a minor edit to the
      commit message to add 7 to the list of values checked.

  Jan Beulich (#4: "vmware: Add VMware provided include file"):
    Either the description is wrong, or the patch is stale.
      stale commit message -- fixed.
    I'd say a file with a single comment line in it would suffice.
      Done.

  Jan Beulich (#5: "xen: Add vmware_port support"):
    Can you explain why a HVM param isn't suitable here?
      Issue with changing QEMU on the fly.
      Andrew Cooper: My recommendation is still to use a creation flag
        So no change.
    Please move SVM's identical definition into ...
      Did this as #1.  No longer needed, but since the patch was ready
      I have included it.
    --Lots of questions about code that no long is part of this patch. --
    With this, is handling other than 32-bit in/out really
    meaningful/correct?
      Added comment about this.
    Since you can't get here for PV, I can't see what you need this.
      Changed to an ASSERT.
    Why version 4?
      Added comment about this.
    -- Several questions about register changes.
      Re-coded to use new_eax and set *val to this.
      Change to generealy use reg->_e..
    These ei1/ei2 checks belong in the callers imo -
      Moved.
    the "port" function parameter isn't even checked
      Add check for exact match.
    If dropping the code is safe without also forbidding the
    combination of nested and VMware emulation.
      Added the forbidding the combination of nested and VMware.
      Mostly do to the cases of the nested virtual code is the one
      to handle VMware stuff if needed, not the root one.  Also I am
      having issues testing xen nested in xen and using hvm.

      

Changes v7 to v8:

  Jan Beulich:
    Coding changes to vmport_ioport. Things like:
-             regs->rax = (uint32_t)~0ul;
+             regs->_eax = ~0u;
      
  Andrew Cooper (#2: "tools: Add vmware_hwver support"):
    Other than these two comments, the rest of the patch looks ok, so...
      Added Reviewed-by after addressing the "Spurious whitepsace change".
      and the wording in the new docs/misc/hypervisor-cpuid.markdown.


Changes v6 to v7:
  summary of changes.

  George Dunlap:
    Any doc about this?
      Added reference to:
        https://sites.google.com/site/chitchatvmback/backdoor
      Last updated: Feb. 2008

  George Dunlap & Jan Beulich
    Too much logging and tracing.
      Dropped a lot of it.  This includes vmport_debug=

  Ian Campbell:
    Any reason RPC code cannot be done in QEMU?
      Not that I know of, so dropped all parts of RPC code.
    Default handling of hvm.vga.kind bad.
      Fixed.
    Default of vmware_port should be based on vmware_hw.
      Done. 

  Tim Deegan:
    CPL check of GETHZ needs to be fixed somewhere.
      Added check for CPL == 0 (assuming this is what VMware is
      checking.  Matches the testing.

  Ian Campbell, Andrew Cooper, George Dunlap, Boris Ostrovsky,
   & Jan Beulich
     Various minor fixes.
    
  Per patch notes:
    #1 "xen: Add support for VMware cpuid leaves":
      Prevent setting of HVM_PARAM_VIRIDIAN if HVM_PARAM_VMWARE_HW set.
    #4 "xen: Add vmware_port support":
      More on AMD in the commit message.
      Switch to only change 32bit part of registers, what VMware
        does.
    #6 "Add xentrace to vmware_port":
      Dropped some of the new traces.
      Added HVMTRACE_ND7.
    #7 "Add xen-hvm-param":
       Was a later patch.  Still optional.
       Fixed formatting.
       Adjust for drop of VMware RPC.

Comments on v3, v4, v5, v6:
  George Dunlap:
    Is there any reason not to merge 05/16 with 03/16?
      The reason I have is that v3 03/16 only contains new files. 2
      from VMware and 1 to allow use of the VMware files.  I added
      xen/arch/x86/hvm/vmware/includeCheck.h at the request of
      Konrad Wilk.

      This patch has many style issues and white space issues.  So I
      want it as a separate patch so as to be clear on what files do
      not meet the coding style.  And why and where they came from.

Changes v5 to v6:
  Boris Ostrovsky & Jan Beulich
    #4 "xen: Add vmware_port support":
    #6 "xen: Convert vmware_port to xentrace usage":
    There is an issue with reading instruction bytes more then once.
      Dropped the attempt to use svm_nextrip_insn_length via
      __get_instruction_length (added in v2).  Just always look
      at upto 15 bytes on AMD.

Changes v4 to v5:
  Re tagged the optional patches.

  Added debug=y build checking that vmx is defining
  VM_EXIT_INTR_ERROR_CODE.

  Boris Ostrovsky:
    #1 "xen: Add support for VMware cpuid leaves":
      Given how is_viridian and is_vmware are defined I think '||' is more
      appropriate.
        Fixed.
    #4 "xen: Add is_vmware_port_enabled":
      we should make sure that svm_vmexit_gp_intercept is not executed for
      any other guest.
        Added an ASSERT on is_vmware_port_enabled.
      magic integers?
        Added #define for them.
    #6 "xen: Convert vmware_port to xentrace usage":
      exitinfo1 is used twice.
        Fixed.
    #7 "tools: Convert vmware_port to xentrace usage":
      'bytes = 0x%(2)d' or 'bytes = %(2)d' ?
        Fixed.
    #8 "xen: Add limited support of VMware's hyper-call rpc":
      PV vs. HVM vs. PVH. So probably 'if(is_hvm_vcpu)'?
        I see no reason to exclude PVH.   Will change to has_hvm_container_vcpu
    #11 "Add live migration of VMware's hyper-call":
      You ASSERTed that vg->key_len is 1 so you may not need the 'if'.
        That is a ASSERT(sizeof, not just ASSERT -- not changed.
      Use real errno, not -1.
        Fixed.
      No ASSERT in vmport_load_domain_ctxt
        Added.

  Jan Beulich & Boris Ostrovsky:
    #8 "xen: Add limited support of VMware's hyper-call rpc":
      The names of all three functions are bogus.
        removed static support routines.
        Also changed in #1.

  Andrew Cooper:
    #2 "tools: Add vmware_hw support":
      Anything looking for Xen according to the Xen cpuid instructions...
        Adjusted doc to new wording.
    #4 "xen: Add is_vmware_port_enabled":
      I am fairly certain that you need some brackets here.
        Added brackets.

  Jan Beulich & Andrew Cooper:
    #1 "xen: Add support for VMware cpuid leaves":
      This hunk is unrelated, but is perhaps something better fixed.
        Added to commit message.
      include <xen/types.h> (IIRC) please.
        Done.
      At least 1 pair of brackets please, especially as the placement of
      brackets affects the result of this particular calculation.
        Switch to "1000000ull / APIC_BUS_CYCLE_NS"      


Changes v3 to v4:
  Ian Campbell:
    Report on both viridian and vmware_hw set.
    Added LIBXL_VGA_INTERFACE_TYPE_VMWARE (vga=vmware).

  Andrew Cooper:
    Add doc for hypervisor-cpuid.

  Boris Ostrovsky:
    Changing regs->error_code may not be a good idea.
      Dropped this.
    
  Jan Beulich & Boris Ostrovsky:
    Only enable vmwxit for GP when vmware_port is set.
      Done.


Changes v2 to v3:

  Add optional unit test tools.
  Re-worked split of changes.

  Jan Beulich:
    for #0:
      I don't think you should be adding a new fine in hvm/ _and_ a new
      subdirectory.
        Moved all files to hvm/vmware that contain code.
    for old #1 (now #1 & #2):
      Is there really a point in enabling both Viridian and VMware extensions?
        I still think so.
      hvmloader change: This needs an explanation
        Dropped as not need now.
      Can you make vmware_hw similar to Viridian, returning success when
      setting the value to what it already is.
        Done.
      You don't seem to be using sub_idx: ...
        Dropped.
      Extra changes...
        Dropped.
    for old #2 (now #3):
      ... these guards have the (theoretical at this point) risk of clashing
      ... the patch is obviously incomplete without this header...
        Did not fix any of these issues.  I will stick with this needs
        to be a 2nd patch that changes the include files to better fit
        in Xen coding.  For now these files are in a sub directory
        which is not part of the normal include search.
        Moved the includeCheck.h file into this patch.
    for old #3 (now #4, #5, #6, #7, #8, #9, #10, #11)
      As I think was said on v1 already - this should be split into smaller
      pieces ...
        Done.
      All this would very likely better go into a separate function placed in
      vmport.c.
        Moved most of the code into vmport.c or vmport_rpc.c.
      In any event I'm rather uncomfortable about vmware_port getting
      enabled unconditionally, ...
        Added vmware_port (done in new patches #4, #5) as an xl.cfg
        option.
      You'll have to go through and fix coding style issues.
        I think I have found all these, but since they do not stand out
        for me, let me know of any left.
      "MAKE_INSTR(IN," name is ambiguous.
        Added all 4 opcodes for in and out that can access this port: INB_DX,
        INL_DX, OUTB_DX, OUTL_DX.
      A VMX-specific function shouldn't be named this way...
        Added new common routine vmport_gp_check() that is called from
        both vmx.c and svm.c which is where all the logic about checking
        for IN ans OUT is done.
        Also fixed naming and added static.
      Ah, here we go (as to using HVM_DBG_LOG()): Isn't this _way_ too
      fine grained?
        I have reduced the number of bits used.  Partialy by switching
        some to xentrace (new patch #6 and #7).
      Right, and zero is an indication that it wasn't found. Also I just
      noticed there's a gdprintk() in that event, which for all other ...
        Made the gdprintk() optional.

End of v3 changes.

This is a small part of the changes needed to allow running Linux
and windows (and others) guests that were built on VMware and run
run them unchanged on Xen.

This small part is the start of Xen support of VMware backdoor I/O
port which is how VMware tools (a standard addition installed on a
guest) communicates to the hypervisor.

I picked this subset to start with because it only has changes in
Xen.

Some of this code is already in QEMU and so KVM has some of this
already.  QEMU supported backdoor commands include VMware mouse
support.  A later patch set exists that links these changes, new
code and Xen changes to QEMU to provide VMware mouse support under
Xen.  The important part is that VMware mouse is an absolute
position mouse and so network delays do not effect usage of the
virtual mouse.

For example from the guest:

[root@C63-min-tools ~]# vmtoolsd --cmd "info-get guestinfo.joejoel"
No value found
[root@C63-min-tools ~]# vmtoolsd --cmd "info-set guestinfo.joejoel short"

[root@C63-min-tools ~]# vmtoolsd --cmd "info-get guestinfo.joejoel"
short
[root@C63-min-tools ~]# vmtoolsd --cmd "info-set guestinfo.joejoel long222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222200000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000joel"

[root@C63-min-tools ~]# vmtoolsd --cmd "info-get guestinfo.key1"
data1
[root@C63-min-tools ~]# vmtoolsd --cmd "info-get guestinfo.key2"
No value found
[root@C63-min-tools ~]# vmtoolsd --cmd "info-get guestinfo.key2"
data2
[root@C63-min-tools ~]# 


Most of this code has been reverse engineered by looking at
source code for Linux and open VMware tools.

http://open-vm-tools.sourceforge.net


changes RFC to v2:

Jan Beulich:
  Add xen/arch/x86/hvm/vmware.c for cpuid_vmware_leaves
  Fewer patches

Andrew Cooper:
  use the proper constant for apic_khz
  Follow 839b966e3f587bbb1a0d954230fb3904330dccb6 style changes.
  Changed HVM_PARAM_VMWARE_HW to write once (make is_vmware_domain()
    more static).
  Dropped vmport status stuff.
  Added checks for xzalloc() having failed.
  You should include backdoor_def.h ...
     Every thing I tried did not work better.  So I did not
     change VMPORT_PORT and BDOOR_PORT being the same value.
     I did not try and adjust VMware's include file backdoor_def.h
     to working in other xen source files.
  Switching to s_time_t is not valid. get_sec() is defined:
    unsigned long get_sec(void);
  and so my uses of it should be using unsigned long.  However
  since that is not a fixed width type, I used the uint64_t
  data type which is almost the same, but does allow the 32 bit
  build of libxc, libxl to do the correct thing.


Konrad Rzeszutek Wilk:
  Please don't include the address. It should be, etc
      about the Vmware provided include files.
    I went with no changes to these files.  Even if the files should
    be changed to match xen coding style, etc I still feel that the
    original ones should be added via a patch, and then adjusted in a
    2nd patch.
  Can you use XenBus?
    I would say no.  XenBus (and XenStore) is about domain to domain
    communication.  This is about VMware's hyper-call and providing
    access to VMware's guest info very low speed access.

Olaf Hering:
   Dropped changing of bios-strings.  Still needs some documentation
   about this may be needed to do in a tool stack or set of commands.


Boris Ostrovsky:
  Use svm_nextrip_insn_length()
    Looks like __get_instruction_length() does this, so switched to
    __get_instruction_length().
 
RFC:

See

http://kb.vmware.com/selfservice/microsites/search.do?language=en_US&cmd=displayKC&externalId=1009458

for info on detecting VMware.

Linux does not follow this exactly.  It checks for CPUID 1st.  If
that fails, it checks for SMBIOS containing "VMware" (not VMware- or
VMW).

So this patch set provides:

        SMBIOS -- Add string VMware-
        CPUID -- Add VMware's CPUID (Note: currently HyperV (viridian support) breaks this check.)
        Add the magic VMware port
            Allow VMware tools poweroff and reboot
            Enable access to VMware's guest info
            Provide the VMware tools build number


Don Slutz (10):
  tools: Add vga=vmware
  xen: Add support for VMware cpuid leaves
  tools: Add vmware_hwver support
  vmware: Add VMware provided include file.
  xen: Add vmware_port support
  xen: Add ring 3 vmware_port support
  tools: Add vmware_port support
  Add IOREQ_TYPE_VMWARE_PORT
  Add xentrace to vmware_port
  test_x86_emulator.c: Add tests for #GP usage

 docs/man/xl.cfg.pod.5                        |  41 +++++-
 tools/libxc/xc_domain.c                      |   2 +-
 tools/libxc/xc_hvm_build_x86.c               |   5 +-
 tools/libxl/libxl.c                          |   4 +-
 tools/libxl/libxl.h                          |  12 ++
 tools/libxl/libxl_create.c                   |  27 +++-
 tools/libxl/libxl_dm.c                       |  12 +-
 tools/libxl/libxl_dom.c                      |   7 +-
 tools/libxl/libxl_internal.h                 |   3 +-
 tools/libxl/libxl_types.idl                  |   3 +
 tools/libxl/libxl_x86.c                      |   5 +-
 tools/libxl/xl_cmdimpl.c                     |  12 +-
 tools/tests/x86_emulator/test_x86_emulator.c | 189 +++++++++++++++++++++++++
 tools/xentrace/formats                       |   5 +
 xen/arch/x86/domain.c                        |   6 +
 xen/arch/x86/hvm/Makefile                    |   1 +
 xen/arch/x86/hvm/emulate.c                   | 132 ++++++++++++++---
 xen/arch/x86/hvm/hvm.c                       | 202 ++++++++++++++++++++++++---
 xen/arch/x86/hvm/io.c                        |  19 +++
 xen/arch/x86/hvm/svm/svm.c                   |  26 ++++
 xen/arch/x86/hvm/svm/vmcb.c                  |   2 +
 xen/arch/x86/hvm/vmware/Makefile             |   2 +
 xen/arch/x86/hvm/vmware/backdoor_def.h       | 167 ++++++++++++++++++++++
 xen/arch/x86/hvm/vmware/cpuid.c              |  75 ++++++++++
 xen/arch/x86/hvm/vmware/includeCheck.h       |   1 +
 xen/arch/x86/hvm/vmware/vmport.c             | 165 ++++++++++++++++++++++
 xen/arch/x86/hvm/vmx/vmcs.c                  |   2 +
 xen/arch/x86/hvm/vmx/vmx.c                   |  37 +++++
 xen/arch/x86/traps.c                         |   8 +-
 xen/arch/x86/x86_emulate/x86_emulate.c       |  13 +-
 xen/arch/x86/x86_emulate/x86_emulate.h       |   5 +
 xen/include/asm-x86/hvm/domain.h             |   9 +-
 xen/include/asm-x86/hvm/emulate.h            |   2 +
 xen/include/asm-x86/hvm/hvm.h                |  10 ++
 xen/include/asm-x86/hvm/trace.h              |  22 +++
 xen/include/asm-x86/hvm/vmware.h             |  33 +++++
 xen/include/public/arch-x86/xen.h            |   6 +-
 xen/include/public/hvm/hvm_op.h              |   5 +
 xen/include/public/hvm/ioreq.h               |  17 +++
 xen/include/public/hvm/params.h              |   4 +-
 xen/include/public/trace.h                   |   3 +
 41 files changed, 1237 insertions(+), 64 deletions(-)
 create mode 100644 xen/arch/x86/hvm/vmware/Makefile
 create mode 100644 xen/arch/x86/hvm/vmware/backdoor_def.h
 create mode 100644 xen/arch/x86/hvm/vmware/cpuid.c
 create mode 100644 xen/arch/x86/hvm/vmware/includeCheck.h
 create mode 100644 xen/arch/x86/hvm/vmware/vmport.c
 create mode 100644 xen/include/asm-x86/hvm/vmware.h

-- 
1.8.4

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v10 01/10] tools: Add vga=vmware
  2015-05-14 23:34 [PATCH v10 00/10] Xen VMware tools support Don Slutz
@ 2015-05-14 23:34 ` Don Slutz
  2015-05-14 23:42   ` Andrew Cooper
  2015-05-14 23:34 ` [PATCH v10 02/10] xen: Add support for VMware cpuid leaves Don Slutz
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Don Slutz @ 2015-05-14 23:34 UTC (permalink / raw)
  To: xen-devel
  Cc: Kevin Tian, Keir Fraser, Ian Campbell, Stefano Stabellini,
	Jun Nakajima, Eddie Dong, Ian Jackson, Don Slutz, Tim Deegan,
	George Dunlap, Aravind Gopalakrishnan, Jan Beulich,
	Andrew Cooper, Boris Ostrovsky, Suravee Suthikulpanit

This allows use of QEMU's VMware emulated video card

Signed-off-by: Don Slutz <dslutz@verizon.com>
---
v10: New at v10.

  Was part of "tools: Add vmware_hwver support"

 docs/man/xl.cfg.pod.5       | 2 +-
 tools/libxl/libxl.h         | 6 ++++++
 tools/libxl/libxl_dm.c      | 8 ++++++++
 tools/libxl/libxl_types.idl | 1 +
 tools/libxl/xl_cmdimpl.c    | 2 ++
 5 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 8e4154f..ba78374 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1374,7 +1374,7 @@ This option is deprecated, use vga="stdvga" instead.
 
 =item B<vga="STRING">
 
-Selects the emulated video card (none|stdvga|cirrus|qxl).
+Selects the emulated video card (none|stdvga|cirrus|qxl|vmware).
 The default is cirrus.
 
 In general, QXL should work with the Spice remote display protocol
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 2ed7194..007a211 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -192,6 +192,12 @@
  * is not present, instead of ERROR_INVAL.
  */
 #define LIBXL_HAVE_ERROR_DOMAIN_NOTFOUND 1
+
+/*
+ * The libxl_vga_interface_type has the type for vmware.
+ */
+#define LIBXL_HAVE_LIBXL_VGA_INTERFACE_TYPE_VMWARE 1
+
 /*
  * libxl ABI compatibility
  *
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 0c6408d..9a06f9b 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -251,6 +251,9 @@ static char ** libxl__build_device_model_args_old(libxl__gc *gc,
         case LIBXL_VGA_INTERFACE_TYPE_NONE:
             flexarray_append_pair(dm_args, "-vga", "none");
             break;
+        case LIBXL_VGA_INTERFACE_TYPE_VMWARE:
+            flexarray_append_pair(dm_args, "-vga", "vmware");
+            break;
         case LIBXL_VGA_INTERFACE_TYPE_QXL:
             break;
         }
@@ -633,6 +636,11 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
                 GCSPRINTF("qxl-vga,vram_size_mb=%"PRIu64",ram_size_mb=%"PRIu64,
                 (b_info->video_memkb/2/1024), (b_info->video_memkb/2/1024) ) );
             break;
+        case LIBXL_VGA_INTERFACE_TYPE_VMWARE:
+            flexarray_append_pair(dm_args, "-device",
+                GCSPRINTF("vmware-svga,vgamem_mb=%d",
+                libxl__sizekb_to_mb(b_info->video_memkb)));
+            break;
         }
 
         if (b_info->u.hvm.boot) {
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 65d479f..9d6ca45 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -184,6 +184,7 @@ libxl_vga_interface_type = Enumeration("vga_interface_type", [
     (2, "STD"),
     (3, "NONE"),
     (4, "QXL"),
+    (5, "VMWARE"),
     ], init_val = "LIBXL_VGA_INTERFACE_TYPE_CIRRUS")
 
 libxl_vendor_device = Enumeration("vendor_device", [
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 373aa37..0e44b12 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2117,6 +2117,8 @@ skip_vfb:
                 b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_NONE;
             } else if (!strcmp(buf, "qxl")) {
                 b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_QXL;
+            } else if (!strcmp(buf, "vmware")) {
+                b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_VMWARE;
             } else {
                 fprintf(stderr, "Unknown vga \"%s\" specified\n", buf);
                 exit(1);
-- 
1.8.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v10 02/10] xen: Add support for VMware cpuid leaves
  2015-05-14 23:34 [PATCH v10 00/10] Xen VMware tools support Don Slutz
  2015-05-14 23:34 ` [PATCH v10 01/10] tools: Add vga=vmware Don Slutz
@ 2015-05-14 23:34 ` Don Slutz
  2015-05-19 20:02   ` Andrew Cooper
  2015-05-14 23:34 ` [PATCH v10 03/10] tools: Add vmware_hwver support Don Slutz
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Don Slutz @ 2015-05-14 23:34 UTC (permalink / raw)
  To: xen-devel
  Cc: Kevin Tian, Keir Fraser, Ian Campbell, Stefano Stabellini,
	Jun Nakajima, Eddie Dong, Ian Jackson, Don Slutz, Tim Deegan,
	George Dunlap, Aravind Gopalakrishnan, Jan Beulich,
	Andrew Cooper, Boris Ostrovsky, Suravee Suthikulpanit

This is done by adding xen_arch_domainconfig vmware_hw. It is set to
the VMware virtual hardware version.

Currently 0, 3-4, 6-11 are good values.  However the
code only checks for == 0 or != 0 or >= 7.

If non-zero then
  Return VMware's cpuid leaves.  If >= 7 return data, else
  return 0.

The support of hypervisor cpuid leaves has not been agreed to.

MicroSoft Hyper-V (AKA viridian) currently must be at 0x40000000.

VMware currently must be at 0x40000000.

KVM currently must be at 0x40000000 (from Seabios).

Xen can be found at the first otherwise unused 0x100 aligned
offset between 0x40000000 and 0x40010000.

http://download.microsoft.com/download/F/B/0/FB0D01A3-8E3A-4F5F-AA59-08C8026D3B8A/requirements-for-implementing-microsoft-hypervisor-interface.docx

http://kb.vmware.com/selfservice/microsites/search.do?language=en_US&cmd=displayKC&externalId=1009458

http://lwn.net/Articles/301888/
  Attempted to get this cleaned up.

So based on this, I picked the order:

Xen at 0x40000000 or
Viridian or VMware at 0x40000000 and Xen at 0x40000100

If both Viridian and VMware selected, report an error.

Since I need to change xen/arch/x86/hvm/Makefile; also add
a newline at end of file.

Signed-off-by: Don Slutz <dslutz@verizon.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
v10:
    Did not add "Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>"
    because of changes here to do things the new way.
  Reword comment message to reflect new way.

v9:
    s/vmware_hw/vmware_hwver/i
    Change -EXDEV to EOPNOTSUPP.
      Done.
    adding another subdirectory: xen/arch/x86/hvm/vmware
    Much will depend on the discussion of the subsequent patches.
      TBD.
    So for versions < 7 there's effectively no CPUID support at all?
      Changed to check at entry.
    The comment /* Params for VMware */ seems wrong...
      Changed to /* emulated VMware Hardware Version */
    Also please use d, not _d in #define is_vmware_domain()
      Changed.  Line is now > 80 characters, so split into 2.

v7:
      Prevent setting of HVM_PARAM_VIRIDIAN if HVM_PARAM_VMWARE_HW set.
v5:
      Given how is_viridian and is_vmware are defined I think '||' is more
      appropriate.
        Fixed.
      The names of all three functions are bogus.
        removed static support routines.
      This hunk is unrelated, but is perhaps something better fixed.
        Added to commit message.
      include <xen/types.h> (IIRC) please.
        Done.
      At least 1 pair of brackets please, especially as the placement of
      brackets affects the result of this particular calculation.
        Switch to "1000000ull / APIC_BUS_CYCLE_NS"      

 xen/arch/x86/domain.c             |  2 ++
 xen/arch/x86/hvm/Makefile         |  1 +
 xen/arch/x86/hvm/hvm.c            | 11 ++++++
 xen/arch/x86/hvm/vmware/Makefile  |  1 +
 xen/arch/x86/hvm/vmware/cpuid.c   | 75 +++++++++++++++++++++++++++++++++++++++
 xen/arch/x86/traps.c              |  8 +++--
 xen/include/asm-x86/hvm/domain.h  |  3 ++
 xen/include/asm-x86/hvm/hvm.h     |  6 ++++
 xen/include/asm-x86/hvm/vmware.h  | 33 +++++++++++++++++
 xen/include/public/arch-x86/xen.h |  2 +-
 10 files changed, 139 insertions(+), 3 deletions(-)
 create mode 100644 xen/arch/x86/hvm/vmware/Makefile
 create mode 100644 xen/arch/x86/hvm/vmware/cpuid.c
 create mode 100644 xen/include/asm-x86/hvm/vmware.h

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 1f1550e..bc3d3a5 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -518,6 +518,8 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
         hvm_funcs.hap_supported &&
         (domcr_flags & DOMCRF_hap);
     d->arch.hvm_domain.mem_sharing_enabled = 0;
+    if ( config )
+        d->arch.hvm_domain.vmware_hwver = config->vmware_hwver;
 
     d->arch.s3_integrity = !!(domcr_flags & DOMCRF_s3_integrity);
 
diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
index 69af47f..284ca75 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 += vmware
 
 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 689e402..05c80e9 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -59,6 +59,7 @@
 #include <asm/hvm/trace.h>
 #include <asm/hvm/nestedhvm.h>
 #include <asm/hvm/event.h>
+#include <asm/hvm/vmware.h>
 #include <asm/mtrr.h>
 #include <asm/apic.h>
 #include <public/sched.h>
@@ -4253,6 +4254,9 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
     if ( cpuid_viridian_leaves(input, eax, ebx, ecx, edx) )
         return;
 
+    if ( cpuid_vmware_leaves(input, eax, ebx, ecx, edx) )
+        return;
+
     if ( cpuid_hypervisor_leaves(input, count, eax, ebx, ecx, edx) )
         return;
 
@@ -5656,6 +5660,13 @@ static int hvm_allow_set_param(struct domain *d,
     {
     /* The following parameters should only be changed once. */
     case HVM_PARAM_VIRIDIAN:
+        /* Disallow if vmware_hwver */
+        if ( d->arch.hvm_domain.vmware_hwver )
+        {
+            rc = -EOPNOTSUPP;
+            break;
+        }
+        /* Fall through */
     case HVM_PARAM_IOREQ_SERVER_PFN:
     case HVM_PARAM_NR_IOREQ_SERVER_PAGES:
         if ( value != 0 && a->value != value )
diff --git a/xen/arch/x86/hvm/vmware/Makefile b/xen/arch/x86/hvm/vmware/Makefile
new file mode 100644
index 0000000..3fb2e0b
--- /dev/null
+++ b/xen/arch/x86/hvm/vmware/Makefile
@@ -0,0 +1 @@
+obj-y += cpuid.o
diff --git a/xen/arch/x86/hvm/vmware/cpuid.c b/xen/arch/x86/hvm/vmware/cpuid.c
new file mode 100644
index 0000000..4839f11
--- /dev/null
+++ b/xen/arch/x86/hvm/vmware/cpuid.c
@@ -0,0 +1,75 @@
+/*
+ * arch/x86/hvm/vmware/cpuid.c
+ *
+ * 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/sched.h>
+
+#include <asm/hvm/hvm.h>
+#include <asm/hvm/vmware.h>
+
+/*
+ * VMware hardware version 7 defines some of these cpuid levels,
+ * below is a brief description about those.
+ *
+ *     Leaf 0x40000000, Hypervisor CPUID information
+ * # EAX: The maximum input value for hypervisor CPUID info (0x40000010).
+ * # EBX, ECX, EDX: Hypervisor vendor ID signature. E.g. "VMwareVMware"
+ *
+ *     Leaf 0x40000010, Timing information.
+ * # EAX: (Virtual) TSC frequency in kHz.
+ * # EBX: (Virtual) Bus (local apic timer) frequency in kHz.
+ * # ECX, EDX: RESERVED
+ */
+
+int cpuid_vmware_leaves(uint32_t idx, uint32_t *eax, uint32_t *ebx,
+                        uint32_t *ecx, uint32_t *edx)
+{
+    struct domain *d = current->domain;
+
+    if ( !is_vmware_domain(d) ||
+         d->arch.hvm_domain.vmware_hwver < 7 )
+        return 0;
+
+    switch ( idx - 0x40000000 )
+    {
+    case 0x0:
+        *eax = 0x40000010;  /* Largest leaf */
+        *ebx = 0x61774d56;  /* "VMwa" */
+        *ecx = 0x4d566572;  /* "reVM" */
+        *edx = 0x65726177;  /* "ware" */
+        break;
+    case 0x10:
+        /* (Virtual) TSC frequency in kHz. */
+        *eax =  d->arch.tsc_khz;
+        /* (Virtual) Bus (local apic timer) frequency in kHz. */
+        *ebx = 1000000ull / APIC_BUS_CYCLE_NS;
+        *ecx = 0;          /* Reserved */
+        *edx = 0;          /* Reserved */
+        break;
+    default:
+        return 0;
+    }
+
+    return 1;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 4b42b2d..8e1c00a 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -750,8 +750,12 @@ int cpuid_hypervisor_leaves( uint32_t idx, uint32_t sub_idx,
                uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx)
 {
     struct domain *d = current->domain;
-    /* Optionally shift out of the way of Viridian architectural leaves. */
-    uint32_t base = is_viridian_domain(d) ? 0x40000100 : 0x40000000;
+    /*
+     * Optionally shift out of the way of Viridian or VMware
+     * architectural leaves.
+     */
+    uint32_t base = is_viridian_domain(d) || is_vmware_domain(d) ?
+        0x40000100 : 0x40000000;
     uint32_t limit, dummy;
 
     idx -= base;
diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
index 0f8b19a..1cb0311 100644
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -109,6 +109,9 @@ struct hvm_domain {
 
     uint64_t              *params;
 
+    /* emulated VMware Hardware Version */
+    uint64_t               vmware_hwver;
+
     /* Memory ranges with pinned cache attributes. */
     struct list_head       pinned_cacheattr_ranges;
 
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index 77eeac5..2965fbb 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -356,6 +356,12 @@ static inline unsigned long hvm_get_shadow_gs_base(struct vcpu *v)
 #define has_viridian_time_ref_count(d) \
     (is_viridian_domain(d) && (viridian_feature_mask(d) & HVMPV_time_ref_count))
 
+#define vmware_feature_mask(d) \
+    ((d)->arch.hvm_domain.vmware_hwver)
+
+#define is_vmware_domain(d) \
+    (is_hvm_domain(d) && vmware_feature_mask(d))
+
 void hvm_hypervisor_cpuid_leaf(uint32_t sub_idx,
                                uint32_t *eax, uint32_t *ebx,
                                uint32_t *ecx, uint32_t *edx);
diff --git a/xen/include/asm-x86/hvm/vmware.h b/xen/include/asm-x86/hvm/vmware.h
new file mode 100644
index 0000000..8390173
--- /dev/null
+++ b/xen/include/asm-x86/hvm/vmware.h
@@ -0,0 +1,33 @@
+/*
+ * asm-x86/hvm/vmware.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. <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef ASM_X86_HVM_VMWARE_H__
+#define ASM_X86_HVM_VMWARE_H__
+
+#include <xen/types.h>
+
+int cpuid_vmware_leaves(uint32_t idx, uint32_t *eax, uint32_t *ebx,
+                        uint32_t *ecx, uint32_t *edx);
+
+#endif /* ASM_X86_HVM_VMWARE_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
index cea3fe7..5a5bad6 100644
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -263,7 +263,7 @@ struct arch_shared_info {
 typedef struct arch_shared_info arch_shared_info_t;
 
 struct xen_arch_domainconfig {
-    char dummy;
+    uint64_t vmware_hwver;
 };
 
 #endif /* !__ASSEMBLY__ */
-- 
1.8.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v10 03/10] tools: Add vmware_hwver support
  2015-05-14 23:34 [PATCH v10 00/10] Xen VMware tools support Don Slutz
  2015-05-14 23:34 ` [PATCH v10 01/10] tools: Add vga=vmware Don Slutz
  2015-05-14 23:34 ` [PATCH v10 02/10] xen: Add support for VMware cpuid leaves Don Slutz
@ 2015-05-14 23:34 ` Don Slutz
  2015-05-14 23:34 ` [PATCH v10 04/10] vmware: Add VMware provided include file Don Slutz
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Don Slutz @ 2015-05-14 23:34 UTC (permalink / raw)
  To: xen-devel
  Cc: Kevin Tian, Keir Fraser, Ian Campbell, Stefano Stabellini,
	Jun Nakajima, Eddie Dong, Ian Jackson, Don Slutz, Tim Deegan,
	George Dunlap, Aravind Gopalakrishnan, Jan Beulich,
	Andrew Cooper, Boris Ostrovsky, Suravee Suthikulpanit

This is used to set xen_arch_domainconfig vmware_hw. It is set to
the emulated VMware virtual hardware version.

Currently 0, 3-4, 6-11 are good values.  However the code only
checks for == 0, != 0, or < 7.

If non-zero then
  default VGA to VMware's VGA.

Signed-off-by: Don Slutz <dslutz@verizon.com>
---
v10:
    LIBXL_HAVE_LIBXL_VGA_INTERFACE_TYPE_VMWARE &
    LIBXL_HAVE_BUILDINFO_HVM_VMWARE_HWVER are arriving together
    a single umbrella could be used.
      Since I split the LIBXL_VGA_INTERFACE_TYPE_VMWARE into
      it's own patch, this is not longer true.
      But I did use 1 for the 2 c_info changes.
    Please use GCSPRINTF.
  Remove vga=vmware from here.

v9:
      I assumed that s/vmware_hw/vmware_hwver/ is not a big enough
      change to drop the Reviewed-by.  Did a minor edit to the
      commit message to add 7 to the list of values checked.

v7:
    Default handling of hvm.vga.kind bad.
      Fixed.
    Default of vmware_port should be based on vmware_hw.
      Done. 

v5:
      Anything looking for Xen according to the Xen cpuid instructions...
        Adjusted doc to new wording.

 docs/man/xl.cfg.pod.5        | 25 ++++++++++++++++++++++++-
 tools/libxc/xc_domain.c      |  2 +-
 tools/libxl/libxl.c          |  4 +++-
 tools/libxl/libxl.h          |  1 +
 tools/libxl/libxl_create.c   | 18 +++++++++++++-----
 tools/libxl/libxl_dm.c       |  2 +-
 tools/libxl/libxl_dom.c      |  7 ++++---
 tools/libxl/libxl_internal.h |  3 ++-
 tools/libxl/libxl_types.idl  |  1 +
 tools/libxl/libxl_x86.c      |  3 +--
 tools/libxl/xl_cmdimpl.c     |  9 ++++++---
 11 files changed, 57 insertions(+), 18 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index ba78374..f62d9f2 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1333,6 +1333,25 @@ The viridian option can be specified as a boolean. A value of true (1)
 is equivalent to the list [ "defaults" ], and a value of false (0) is
 equivalent to an empty list.
 
+=item B<vmware_hwver=NUMBER>
+
+Turns on or off the exposure of VMware cpuid.  The number is
+VMware's hardware version number, where 0 is off.  A number >= 7
+is needed to enable exposure of VMware cpuid.
+
+If not zero it changes the default VGA to VMware's VGA.
+
+The hardware version number (vmware_hwver) come from VMware config files.
+
+=over 4
+
+In a .vmx it is virtualHW.version
+
+In a .ovf it is part of the value of vssd:VirtualSystemType.
+For vssd:VirtualSystemType == vmx-07, vmware_hwver = 7.
+
+=back
+
 =back
 
 =head3 Emulated VGA Graphics Device
@@ -1372,10 +1391,14 @@ later (e.g. Windows XP onwards) then you should enable this.
 stdvga supports more video ram and bigger resolutions than Cirrus.
 This option is deprecated, use vga="stdvga" instead.
 
+The deprecated B<stdvga=0> prevents the usage of vmware by default
+if B<vmware_hwver> is non-zero.
+
 =item B<vga="STRING">
 
 Selects the emulated video card (none|stdvga|cirrus|qxl|vmware).
-The default is cirrus.
+The default is cirrus unless B<vmware_hwver> is non-zero in which case it
+is vmware.
 
 In general, QXL should work with the Spice remote display protocol
 for acceleration, and QXL driver is necessary in guest in this case.
diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index a7079a1..40ff6ba 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -64,7 +64,7 @@ int xc_domain_create(xc_interface *xch,
     memset(&config, 0, sizeof(config));
 
 #if defined (__i386) || defined(__x86_64__)
-    /* No arch-specific configuration for now */
+    /* No arch-specific default configuration for now */
 #elif defined (__arm__) || defined(__aarch64__)
     config.gic_version = XEN_DOMCTL_CONFIG_GIC_DEFAULT;
     config.nr_spis = 0;
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index a6eb2df..c154246 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -4930,12 +4930,14 @@ int libxl_get_memory_target(libxl_ctx *ctx, uint32_t domid,
 }
 
 int libxl_domain_need_memory(libxl_ctx *ctx, libxl_domain_build_info *b_info,
+                             libxl_domain_create_info *c_info,
                              uint32_t *need_memkb)
 {
     GC_INIT(ctx);
     int rc;
 
-    rc = libxl__domain_build_info_setdefault(gc, b_info);
+    rc = libxl__domain_build_info_setdefault(gc, b_info,
+                                             c_info->vmware_hwver != 0);
     if (rc) goto out;
 
     *need_memkb = b_info->target_memkb;
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 007a211..61d89be 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1099,6 +1099,7 @@ int libxl_get_memory_target(libxl_ctx *ctx, uint32_t domid, uint32_t *out_target
  */
 /* how much free memory in the system a domain needs to be built */
 int libxl_domain_need_memory(libxl_ctx *ctx, libxl_domain_build_info *b_info,
+                             libxl_domain_create_info *c_info,
                              uint32_t *need_memkb);
 /* how much free memory is available in the system */
 int libxl_get_free_memory(libxl_ctx *ctx, uint32_t *memkb);
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index f0da7dc..b7818bc 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -101,7 +101,8 @@ static int sched_params_valid(libxl__gc *gc,
 }
 
 int libxl__domain_build_info_setdefault(libxl__gc *gc,
-                                        libxl_domain_build_info *b_info)
+                                        libxl_domain_build_info *b_info,
+                                        bool vmware_vga_default)
 {
     int i;
 
@@ -238,8 +239,12 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
         if (b_info->u.hvm.mmio_hole_memkb == LIBXL_MEMKB_DEFAULT)
             b_info->u.hvm.mmio_hole_memkb = 0;
 
-        if (!b_info->u.hvm.vga.kind)
-            b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+        if (!b_info->u.hvm.vga.kind) {
+            if (vmware_vga_default)
+                b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_VMWARE;
+            else
+                b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+        }
 
         switch (b_info->device_model_version) {
         case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
@@ -455,7 +460,7 @@ int libxl__domain_build(libxl__gc *gc,
         vments[4] = "start_time";
         vments[5] = libxl__sprintf(gc, "%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
 
-        localents = libxl__calloc(gc, 9, sizeof(char *));
+        localents = libxl__calloc(gc, 11, sizeof(char *));
         i = 0;
         localents[i++] = "platform/acpi";
         localents[i++] = libxl_defbool_val(info->u.hvm.acpi) ? "1" : "0";
@@ -463,6 +468,8 @@ int libxl__domain_build(libxl__gc *gc,
         localents[i++] = libxl_defbool_val(info->u.hvm.acpi_s3) ? "1" : "0";
         localents[i++] = "platform/acpi_s4";
         localents[i++] = libxl_defbool_val(info->u.hvm.acpi_s4) ? "1" : "0";
+        localents[i++] = "platform/vmware_hwver";
+        localents[i++] = GCSPRINTF("%"PRId64, d_config->c_info.vmware_hwver);
         if (info->u.hvm.mmio_hole_memkb) {
             uint64_t max_ram_below_4g =
                 (1ULL << 32) - (info->u.hvm.mmio_hole_memkb << 10);
@@ -901,7 +908,8 @@ static void initiate_domain_create(libxl__egc *egc,
     dcs->guest_domid = domid;
     dcs->dmss.dm.guest_domid = 0; /* means we haven't spawned */
 
-    ret = libxl__domain_build_info_setdefault(gc, &d_config->b_info);
+    ret = libxl__domain_build_info_setdefault(gc, &d_config->b_info,
+                                   d_config->c_info.vmware_hwver != 0);
     if (ret) goto error_out;
 
     if (!sched_params_valid(gc, domid, &d_config->b_info.sched_params)) {
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 9a06f9b..c04fa0d 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1099,7 +1099,7 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
 
     ret = libxl__domain_create_info_setdefault(gc, &dm_config->c_info);
     if (ret) goto out;
-    ret = libxl__domain_build_info_setdefault(gc, &dm_config->b_info);
+    ret = libxl__domain_build_info_setdefault(gc, &dm_config->b_info, false);
     if (ret) goto out;
 
     GCNEW(vfb);
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index a0c9850..f7add33 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -131,7 +131,8 @@ static int numa_cmpf(const libxl__numa_candidate *c1,
 
 /* The actual automatic NUMA placement routine */
 static int numa_place_domain(libxl__gc *gc, uint32_t domid,
-                             libxl_domain_build_info *info)
+                             libxl_domain_build_info *info,
+                             libxl_domain_create_info *c_info)
 {
     int found;
     libxl__numa_candidate candidate;
@@ -155,7 +156,7 @@ static int numa_place_domain(libxl__gc *gc, uint32_t domid,
     if (rc)
         return rc;
 
-    rc = libxl_domain_need_memory(CTX, info, &memkb);
+    rc = libxl_domain_need_memory(CTX, info, c_info, &memkb);
     if (rc)
         goto out;
     if (libxl_node_bitmap_alloc(CTX, &cpupool_nodemap, 0)) {
@@ -353,7 +354,7 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
         if (rc)
             return rc;
 
-        rc = numa_place_domain(gc, domid, info);
+        rc = numa_place_domain(gc, domid, info, &d_config->c_info);
         if (rc) {
             libxl_bitmap_dispose(&cpumap_soft);
             return rc;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 057fd0f..eff6bc0 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1099,7 +1099,8 @@ _hidden int libxl__init_console_from_channel(libxl__gc *gc,
 _hidden int libxl__domain_create_info_setdefault(libxl__gc *gc,
                                         libxl_domain_create_info *c_info);
 _hidden int libxl__domain_build_info_setdefault(libxl__gc *gc,
-                                        libxl_domain_build_info *b_info);
+                                        libxl_domain_build_info *b_info,
+                                        bool vmware_vga_default);
 _hidden int libxl__device_disk_setdefault(libxl__gc *gc,
                                           libxl_device_disk *disk);
 _hidden int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 9d6ca45..501bb48 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -343,6 +343,7 @@ libxl_domain_create_info = Struct("domain_create_info",[
     ("run_hotplug_scripts",libxl_defbool),
     ("pvh",          libxl_defbool),
     ("driver_domain",libxl_defbool),
+    ("vmware_hwver", uint64),
     ], dir=DIR_IN)
 
 libxl_domain_restore_params = Struct("domain_restore_params", [
diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
index ed2bd38..fd7dafa 100644
--- a/tools/libxl/libxl_x86.c
+++ b/tools/libxl/libxl_x86.c
@@ -5,8 +5,7 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
                                       libxl_domain_config *d_config,
                                       xc_domain_configuration_t *xc_config)
 {
-    /* No specific configuration right now */
-
+    xc_config->vmware_hwver = d_config->c_info.vmware_hwver;
     return 0;
 }
 
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 0e44b12..18ba70f 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1382,6 +1382,8 @@ static void parse_config_data(const char *config_source,
     b_info->cmdline = parse_cmdline(config);
 
     xlu_cfg_get_defbool(config, "driver_domain", &c_info->driver_domain, 0);
+    if (!xlu_cfg_get_long(config, "vmware_hwver",  &l, 1))
+        c_info->vmware_hwver = l;
 
     switch(b_info->type) {
     case LIBXL_DOMAIN_TYPE_HVM:
@@ -2395,7 +2397,8 @@ static int preserve_domain(uint32_t *r_domid, libxl_event *event,
     return rc == 0 ? 1 : 0;
 }
 
-static int freemem(uint32_t domid, libxl_domain_build_info *b_info)
+static int freemem(uint32_t domid, libxl_domain_build_info *b_info,
+                   libxl_domain_create_info *c_info)
 {
     int rc, retries = 3;
     uint32_t need_memkb, free_memkb;
@@ -2403,7 +2406,7 @@ static int freemem(uint32_t domid, libxl_domain_build_info *b_info)
     if (!autoballoon)
         return 0;
 
-    rc = libxl_domain_need_memory(ctx, b_info, &need_memkb);
+    rc = libxl_domain_need_memory(ctx, b_info, c_info, &need_memkb);
     if (rc < 0)
         return rc;
 
@@ -2683,7 +2686,7 @@ start:
     if (rc < 0)
         goto error_out;
 
-    ret = freemem(domid, &d_config.b_info);
+    ret = freemem(domid, &d_config.b_info, &d_config.c_info);
     if (ret < 0) {
         fprintf(stderr, "failed to free memory for the domain\n");
         ret = ERROR_FAIL;
-- 
1.8.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v10 04/10] vmware: Add VMware provided include file.
  2015-05-14 23:34 [PATCH v10 00/10] Xen VMware tools support Don Slutz
                   ` (2 preceding siblings ...)
  2015-05-14 23:34 ` [PATCH v10 03/10] tools: Add vmware_hwver support Don Slutz
@ 2015-05-14 23:34 ` Don Slutz
  2015-05-14 23:34 ` [PATCH v10 05/10] xen: Add vmware_port support Don Slutz
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Don Slutz @ 2015-05-14 23:34 UTC (permalink / raw)
  To: xen-devel
  Cc: Kevin Tian, Keir Fraser, Ian Campbell, Stefano Stabellini,
	Jun Nakajima, Eddie Dong, Ian Jackson, Don Slutz, Tim Deegan,
	George Dunlap, Aravind Gopalakrishnan, Jan Beulich,
	Andrew Cooper, Boris Ostrovsky, Suravee Suthikulpanit

This file: backdoor_def.h comes from:

http://packages.vmware.com/tools/esx/3.5latest/rhel4/SRPMS/index.html
 open-vm-tools-kmod-7.4.8-396269.423167.src.rpm
  open-vm-tools-kmod-7.4.8.tar.gz
   vmhgfs/backdoor_def.h

and is unchanged.

Added the badly named include file includeCheck.h also.  It only has
a comment and is provided so that backdoor_def.h can be used without
change.

Signed-off-by: Don Slutz <dslutz@verizon.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
v10:
   Add Acked-by: Andrew Cooper

v9:
    Either the description is wrong, or the patch is stale.
      stale commit message -- fixed.
    I'd say a file with a single comment line in it would suffice.
      Done.


 xen/arch/x86/hvm/vmware/backdoor_def.h | 167 +++++++++++++++++++++++++++++++++
 xen/arch/x86/hvm/vmware/includeCheck.h |   1 +
 2 files changed, 168 insertions(+)
 create mode 100644 xen/arch/x86/hvm/vmware/backdoor_def.h
 create mode 100644 xen/arch/x86/hvm/vmware/includeCheck.h

diff --git a/xen/arch/x86/hvm/vmware/backdoor_def.h b/xen/arch/x86/hvm/vmware/backdoor_def.h
new file mode 100644
index 0000000..e76795f
--- /dev/null
+++ b/xen/arch/x86/hvm/vmware/backdoor_def.h
@@ -0,0 +1,167 @@
+/* **********************************************************
+ * Copyright 1998 VMware, Inc.  All rights reserved. 
+ * **********************************************************
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation version 2 and no later version.
+ *
+ * This program 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.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
+ */
+
+/*
+ * backdoor_def.h --
+ *
+ * This contains backdoor defines that can be included from
+ * an assembly language file.
+ */
+
+
+
+#ifndef _BACKDOOR_DEF_H_
+#define _BACKDOOR_DEF_H_
+
+#define INCLUDE_ALLOW_MODULE
+#define INCLUDE_ALLOW_USERLEVEL
+#define INCLUDE_ALLOW_VMMEXT
+#define INCLUDE_ALLOW_VMCORE
+#define INCLUDE_ALLOW_VMKERNEL
+#include "includeCheck.h"
+
+/*
+ * If you want to add a new low-level backdoor call for a guest userland
+ * application, please consider using the GuestRpc mechanism instead. --hpreg
+ */
+
+#define BDOOR_MAGIC 0x564D5868
+
+/* Low-bandwidth backdoor port. --hpreg */
+
+#define BDOOR_PORT 0x5658
+
+#define BDOOR_CMD_GETMHZ      		   1
+/*
+ * BDOOR_CMD_APMFUNCTION is used by:
+ *
+ * o The FrobOS code, which instead should either program the virtual chipset
+ *   (like the new BIOS code does, matthias offered to implement that), or not
+ *   use any VM-specific code (which requires that we correctly implement
+ *   "power off on CLI HLT" for SMP VMs, boris offered to implement that)
+ *
+ * o The old BIOS code, which will soon be jettisoned
+ *
+ *  --hpreg
+ */
+#define BDOOR_CMD_APMFUNCTION 		   2
+#define BDOOR_CMD_GETDISKGEO  		   3
+#define BDOOR_CMD_GETPTRLOCATION	      4
+#define BDOOR_CMD_SETPTRLOCATION	      5
+#define BDOOR_CMD_GETSELLENGTH		   6
+#define BDOOR_CMD_GETNEXTPIECE		   7
+#define BDOOR_CMD_SETSELLENGTH		   8
+#define BDOOR_CMD_SETNEXTPIECE		   9
+#define BDOOR_CMD_GETVERSION		      10
+#define BDOOR_CMD_GETDEVICELISTELEMENT	11
+#define BDOOR_CMD_TOGGLEDEVICE		   12
+#define BDOOR_CMD_GETGUIOPTIONS		   13
+#define BDOOR_CMD_SETGUIOPTIONS		   14
+#define BDOOR_CMD_GETSCREENSIZE		   15
+#define BDOOR_CMD_MONITOR_CONTROL       16
+#define BDOOR_CMD_GETHWVERSION          17
+#define BDOOR_CMD_OSNOTFOUND            18
+#define BDOOR_CMD_GETUUID               19
+#define BDOOR_CMD_GETMEMSIZE            20
+#define BDOOR_CMD_HOSTCOPY              21 /* Devel only */
+/* BDOOR_CMD_GETOS2INTCURSOR, 22, is very old and defunct. Reuse. */
+#define BDOOR_CMD_GETTIME               23 /* Deprecated. Use GETTIMEFULL. */
+#define BDOOR_CMD_STOPCATCHUP           24
+#define BDOOR_CMD_PUTCHR	        25 /* Devel only */
+#define BDOOR_CMD_ENABLE_MSG	        26 /* Devel only */
+#define BDOOR_CMD_GOTO_TCL	        27 /* Devel only */
+#define BDOOR_CMD_INITPCIOPROM		28
+#define BDOOR_CMD_INT13			29
+#define BDOOR_CMD_MESSAGE               30
+#define BDOOR_CMD_RSVD0                 31
+#define BDOOR_CMD_RSVD1                 32
+#define BDOOR_CMD_RSVD2                 33
+#define BDOOR_CMD_ISACPIDISABLED	34
+#define BDOOR_CMD_TOE			35 /* Not in use */
+/* BDOOR_CMD_INITLSIOPROM, 36, was merged with 28. Reuse. */
+#define BDOOR_CMD_PATCH_SMBIOS_STRUCTS  37
+#define BDOOR_CMD_MAPMEM                38 /* Devel only */
+#define BDOOR_CMD_ABSPOINTER_DATA	39
+#define BDOOR_CMD_ABSPOINTER_STATUS	40
+#define BDOOR_CMD_ABSPOINTER_COMMAND	41
+#define BDOOR_CMD_TIMER_SPONGE          42
+#define BDOOR_CMD_PATCH_ACPI_TABLES	43
+/* Catch-all to allow synchronous tests */
+#define BDOOR_CMD_DEVEL_FAKEHARDWARE	44 /* Debug only - needed in beta */
+#define BDOOR_CMD_GETHZ      		45
+#define BDOOR_CMD_GETTIMEFULL           46
+#define BDOOR_CMD_STATELOGGER           47
+#define BDOOR_CMD_CHECKFORCEBIOSSETUP	48
+#define BDOOR_CMD_LAZYTIMEREMULATION    49
+#define BDOOR_CMD_BIOSBBS               50
+#define BDOOR_CMD_MAX                   51
+
+/* 
+ * IMPORTANT NOTE: When modifying the behavior of an existing backdoor command,
+ * you must adhere to the semantics expected by the oldest Tools who use that
+ * command. Specifically, do not alter the way in which the command modifies 
+ * the registers. Otherwise backwards compatibility will suffer.
+ */
+
+/* High-bandwidth backdoor port. --hpreg */
+
+#define BDOORHB_PORT 0x5659
+
+#define BDOORHB_CMD_MESSAGE 0
+#define BDOORHB_CMD_MAX 1
+
+/*
+ * There is another backdoor which allows access to certain TSC-related
+ * values using otherwise illegal PMC indices when the pseudo_perfctr
+ * control flag is set.
+ */
+
+#define BDOOR_PMC_HW_TSC      0x10000
+#define BDOOR_PMC_REAL_NS     0x10001
+#define BDOOR_PMC_APPARENT_NS 0x10002
+
+#define IS_BDOOR_PMC(index)  (((index) | 3) == 0x10003)
+#define BDOOR_CMD(ecx)       ((ecx) & 0xffff)
+
+
+#ifdef VMM
+/*
+ *----------------------------------------------------------------------
+ *
+ * Backdoor_CmdRequiresFullyValidVCPU --
+ *
+ *    A few backdoor commands require the full VCPU to be valid
+ *    (including GDTR, IDTR, TR and LDTR). The rest get read/write
+ *    access to GPRs and read access to Segment registers (selectors).
+ *
+ * Result:
+ *    True iff VECX contains a command that require the full VCPU to
+ *    be valid.
+ *
+ *----------------------------------------------------------------------
+ */
+static INLINE Bool
+Backdoor_CmdRequiresFullyValidVCPU(unsigned cmd)
+{
+   return cmd == BDOOR_CMD_RSVD0 ||
+          cmd == BDOOR_CMD_RSVD1 ||
+          cmd == BDOOR_CMD_RSVD2;
+}
+#endif
+
+#endif
diff --git a/xen/arch/x86/hvm/vmware/includeCheck.h b/xen/arch/x86/hvm/vmware/includeCheck.h
new file mode 100644
index 0000000..3b63fa4
--- /dev/null
+++ b/xen/arch/x86/hvm/vmware/includeCheck.h
@@ -0,0 +1 @@
+/* Nothing here.  Just to use backdoor_def.h without change. */
-- 
1.8.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v10 05/10] xen: Add vmware_port support
  2015-05-14 23:34 [PATCH v10 00/10] Xen VMware tools support Don Slutz
                   ` (3 preceding siblings ...)
  2015-05-14 23:34 ` [PATCH v10 04/10] vmware: Add VMware provided include file Don Slutz
@ 2015-05-14 23:34 ` Don Slutz
  2015-05-19 20:23   ` Andrew Cooper
  2015-05-14 23:34 ` [PATCH v10 06/10] xen: Add ring 3 " Don Slutz
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Don Slutz @ 2015-05-14 23:34 UTC (permalink / raw)
  To: xen-devel
  Cc: Kevin Tian, Keir Fraser, Ian Campbell, Stefano Stabellini,
	Jun Nakajima, Eddie Dong, Ian Jackson, Don Slutz, Tim Deegan,
	George Dunlap, Aravind Gopalakrishnan, Jan Beulich,
	Andrew Cooper, Boris Ostrovsky, Suravee Suthikulpanit

This includes adding is_vmware_port_enabled

This is a new xen_arch_domainconfig flag,
XEN_DOMCTL_CONFIG_VMWARE_PORT_MASK.

This enables limited support of VMware's hyper-call.

This is both a more complete support then in currently provided by
QEMU and/or KVM and less.  The missing part requires QEMU changes
and has been left out until the QEMU patches are accepted upstream.

VMware's hyper-call is also known as VMware Backdoor I/O Port.

Note: this support does not depend on vmware_hw being non-zero.

Summary is that VMware treats "in (%dx),%eax" (or "out %eax,(%dx)")
to port 0x5658 specially.  Note: since many operations return data
in EAX, "in (%dx),%eax" is the one to use.  The other lengths like
"in (%dx),%al" will still do things, only AL part of EAX will be
changed.  For "out %eax,(%dx)" of all lengths, EAX will remain
unchanged.

An open source example of using this is:

http://open-vm-tools.sourceforge.net/

Which only uses "inl (%dx)".  Also

http://kb.vmware.com/selfservice/microsites/search.do?language=en_US&cmd=displayKC&externalId=1009458

Some of the best info is at:

https://sites.google.com/site/chitchatvmback/backdoor

Signed-off-by: Don Slutz <dslutz@verizon.com>
---
v10:
    Probably better as EOPNOTSUPP, as it is a configuration problem.
    This function looks as if it should be static.
    I would suggest putting vmport_register declaration in hvm.h ...
    As indicated before, I don't think this is a good use case for a
    domain creation flag.
      Switch to the new config way.
    struct domain *d => struct domain *currd
    Are you sure you don't want to zero the high halves of 64-bit ...
      Comment added.
   Then just have this handled into the default case.
      Reworked new_eax handling.
   is_hvm_domain(currd)
   And - why here rather than before the switch() or even right at the
   start of the function?
      Moved to start.
   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.
     All done in comment at start.


v9:
  Switch to x86_emulator to handle #GP code moved to next patch.
    Can you explain why a HVM param isn't suitable here?
      Issue with changing QEMU on the fly.
      Andrew Cooper: My recommendation is still to use a creation flag
        So no change.
    Please move SVM's identical definition into ...
      Did this as #1.  No longer needed, but since the patch was ready
      I have included it.
    --Lots of questions about code that no long is part of this patch. --
    With this, is handling other than 32-bit in/out really
    meaningful/correct?
      Added comment about this.
    Since you can't get here for PV, I can't see what you need this.
      Changed to an ASSERT.
    Why version 4?
      Added comment about this.
    -- Several questions about register changes.
      Re-coded to use new_eax and set *val to this.
      Change to generealy use reg->_e..
    These ei1/ei2 checks belong in the callers imo -
      Moved.
    the "port" function parameter isn't even checked
      Add check for exact match.
    If dropping the code is safe without also forbidding the
    combination of nested and VMware emulation.
      Added the forbidding the combination of nested and VMware.
      Mostly do to the cases of the nested virtual code is the one
      to handle VMware stuff if needed, not the root one.  Also I am
      having issues testing xen nested in xen and using hvm.

v7:
      More on AMD in the commit message.
      Switch to only change 32bit part of registers, what VMware
        does.
    Too much logging and tracing.
      Dropped a lot of it.  This includes vmport_debug=

v6:
      Dropped the attempt to use svm_nextrip_insn_length via
      __get_instruction_length (added in v2).  Just always look
      at upto 15 bytes on AMD.

v5:
      we should make sure that svm_vmexit_gp_intercept is not executed for
      any other guest.
        Added an ASSERT on is_vmware_port_enabled.
      magic integers?
        Added #define for them.
      I am fairly certain that you need some brackets here.
        Added brackets.

 xen/arch/x86/domain.c             |   4 ++
 xen/arch/x86/hvm/hvm.c            |   9 +++
 xen/arch/x86/hvm/vmware/Makefile  |   1 +
 xen/arch/x86/hvm/vmware/vmport.c  | 143 ++++++++++++++++++++++++++++++++++++++
 xen/include/asm-x86/hvm/domain.h  |   3 +
 xen/include/asm-x86/hvm/hvm.h     |   2 +
 xen/include/public/arch-x86/xen.h |   4 ++
 7 files changed, 166 insertions(+)
 create mode 100644 xen/arch/x86/hvm/vmware/vmport.c

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index bc3d3a5..153048a 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -519,7 +519,11 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
         (domcr_flags & DOMCRF_hap);
     d->arch.hvm_domain.mem_sharing_enabled = 0;
     if ( config )
+    {
         d->arch.hvm_domain.vmware_hwver = config->vmware_hwver;
+        d->arch.hvm_domain.is_vmware_port_enabled =
+            !!(config->arch_flags & XEN_DOMCTL_CONFIG_VMWARE_PORT_MASK);
+    }
 
     d->arch.s3_integrity = !!(domcr_flags & DOMCRF_s3_integrity);
 
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 05c80e9..a179123 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1462,6 +1462,9 @@ int hvm_domain_initialise(struct domain *d)
         goto fail1;
     d->arch.hvm_domain.io_handler->num_slot = 0;
 
+    if ( d->arch.hvm_domain.is_vmware_port_enabled )
+        vmport_register(d);
+
     if ( is_pvh_domain(d) )
     {
         register_portio_handler(d, 0, 0x10003, handle_pvh_io);
@@ -5785,6 +5788,12 @@ static int hvmop_set_param(
             break;
         if ( a.value > 1 )
             rc = -EINVAL;
+        /* Prevent nestedhvm with vmport */
+        if ( d->arch.hvm_domain.is_vmware_port_enabled )
+        {
+            rc = -EOPNOTSUPP;
+            break;
+        }
         /*
          * Remove the check below once we have
          * shadow-on-shadow.
diff --git a/xen/arch/x86/hvm/vmware/Makefile b/xen/arch/x86/hvm/vmware/Makefile
index 3fb2e0b..cd8815b 100644
--- a/xen/arch/x86/hvm/vmware/Makefile
+++ b/xen/arch/x86/hvm/vmware/Makefile
@@ -1 +1,2 @@
 obj-y += cpuid.o
+obj-y += vmport.o
diff --git a/xen/arch/x86/hvm/vmware/vmport.c b/xen/arch/x86/hvm/vmware/vmport.c
new file mode 100644
index 0000000..08ddef9
--- /dev/null
+++ b/xen/arch/x86/hvm/vmware/vmport.c
@@ -0,0 +1,143 @@
+/*
+ * 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 "backdoor_def.h"
+
+static int vmport_ioport(int dir, uint32_t port, uint32_t bytes, uint32_t *val)
+{
+    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 for eax.
+     */
+    if ( port == BDOOR_PORT && regs->_eax == BDOOR_MAGIC )
+    {
+        uint32_t new_eax = ~0u;
+        uint64_t value;
+        struct vcpu *curr = current;
+        struct domain *currd = curr->domain;
+
+        ASSERT(is_hvm_domain(currd));
+        /*
+         * VMware changes the other (non eax) registers ignoring dir
+         * (IN vs OUT).  It also changes only the 32-bit part
+         * leaving the high 32-bits unchanged, unlike what one would
+         * expect to happen.
+         */
+        switch ( regs->_ecx & 0xffff )
+        {
+        case BDOOR_CMD_GETMHZ:
+            new_eax = currd->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;
+            break;
+        case BDOOR_CMD_GETHWVERSION:
+            /* vmware_hw */
+            new_eax = currd->arch.hvm_domain.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 = currd->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(currd) -
+                currd->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 = currd->time_offset_seconds / 60;
+            break;
+        case BDOOR_CMD_GETTIMEFULL:
+            /* BDOOR_MAGIC */
+            new_eax = BDOOR_MAGIC;
+            value = get_localtime_us(currd) -
+                currd->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;
+        default:
+            /* Let backing DM handle */
+            return X86EMUL_UNHANDLEABLE;
+        }
+        if ( dir == IOREQ_READ )
+            *val = new_eax;
+    }
+    else if ( dir == IOREQ_READ )
+        *val = ~0u;
+
+    return X86EMUL_OKAY;
+}
+
+void vmport_register(struct domain *d)
+{
+    register_portio_handler(d, BDOOR_PORT, 4, vmport_ioport);
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-set-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
index 1cb0311..d3166e4 100644
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -124,6 +124,9 @@ struct hvm_domain {
     spinlock_t             uc_lock;
     bool_t                 is_in_uc_mode;
 
+    /* VMware backdoor port available */
+    bool_t                 is_vmware_port_enabled;
+
     /* Pass-through */
     struct hvm_iommu       hvm_iommu;
 
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index 2965fbb..e76f612 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -522,6 +522,8 @@ extern bool_t opt_hvm_fep;
 #define opt_hvm_fep 0
 #endif
 
+void vmport_register(struct domain *d);
+
 #endif /* __ASM_X86_HVM_HVM_H__ */
 
 /*
diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
index 5a5bad6..ccc5f1f 100644
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -262,8 +262,12 @@ struct arch_shared_info {
 };
 typedef struct arch_shared_info arch_shared_info_t;
 
+/* Enable use of vmware backdoor port. */
+#define XEN_DOMCTL_CONFIG_VMWARE_PORT_BIT   0
+#define XEN_DOMCTL_CONFIG_VMWARE_PORT_MASK  (1U << XEN_DOMCTL_CONFIG_VMWARE_PORT_BIT)
 struct xen_arch_domainconfig {
     uint64_t vmware_hwver;
+    uint64_t arch_flags;
 };
 
 #endif /* !__ASSEMBLY__ */
-- 
1.8.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v10 06/10] xen: Add ring 3 vmware_port support
  2015-05-14 23:34 [PATCH v10 00/10] Xen VMware tools support Don Slutz
                   ` (4 preceding siblings ...)
  2015-05-14 23:34 ` [PATCH v10 05/10] xen: Add vmware_port support Don Slutz
@ 2015-05-14 23:34 ` Don Slutz
  2015-05-14 23:34 ` [PATCH v10 07/10] tools: Add " Don Slutz
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Don Slutz @ 2015-05-14 23:34 UTC (permalink / raw)
  To: xen-devel
  Cc: Kevin Tian, Keir Fraser, Ian Campbell, Stefano Stabellini,
	Jun Nakajima, Eddie Dong, Ian Jackson, Don Slutz, Tim Deegan,
	George Dunlap, Aravind Gopalakrishnan, Jan Beulich,
	Andrew Cooper, Boris Ostrovsky, Suravee Suthikulpanit

Summary is that VMware treats "in (%dx),%eax" (or "out %eax,(%dx)")
to port 0x5658 specially.  Note: since many operations return data
in EAX, "in (%dx),%eax" is the one to use.  The other lengths like
"in (%dx),%al" will still do things, only AL part of EAX will be
changed.  For "out %eax,(%dx)" of all lengths, EAX will remain
unchanged.

This instruction is allowed to be used from ring 3.  To
support this the vmexit for GP needs to be enabled.  I have not
fully tested that nested HVM is doing the right thing for this.

Enable no-fault of pio in x86_emulate for VMware port

Also adjust the emulation registers after doing a VMware
backdoor operation.

Add new routine hvm_emulate_one_gp() to be used by the #GP fault
handler.

Some of the best info is at:

https://sites.google.com/site/chitchatvmback/backdoor

Signed-off-by: Don Slutz <dslutz@verizon.com>
---
v10:
   Re-worked to be simpler.

v9:
   Split #GP handling (or skipping of #GP) code out of previous
   patch to help with the review process.
   Switch to x86_emulator to handle #GP
   I think the hvm_emulate_ops_gp() covers all needed ops.  Not able to validate
   all paths though _hvm_emulate_one().

 xen/arch/x86/hvm/emulate.c             | 54 ++++++++++++++++++++++++++++++++--
 xen/arch/x86/hvm/svm/svm.c             | 26 ++++++++++++++++
 xen/arch/x86/hvm/svm/vmcb.c            |  2 ++
 xen/arch/x86/hvm/vmware/vmport.c       | 11 +++++++
 xen/arch/x86/hvm/vmx/vmcs.c            |  2 ++
 xen/arch/x86/hvm/vmx/vmx.c             | 37 +++++++++++++++++++++++
 xen/arch/x86/x86_emulate/x86_emulate.c | 13 +++++++-
 xen/arch/x86/x86_emulate/x86_emulate.h |  5 ++++
 xen/include/asm-x86/hvm/emulate.h      |  2 ++
 xen/include/asm-x86/hvm/hvm.h          |  1 +
 10 files changed, 150 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index ac9c9d6..d5e6468 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -803,6 +803,27 @@ static int hvmemul_wbinvd_discard(
     return X86EMUL_OKAY;
 }
 
+static int hvmemul_write_gp(
+    unsigned int seg,
+    unsigned long offset,
+    void *p_data,
+    unsigned int bytes,
+    struct x86_emulate_ctxt *ctxt)
+{
+    return X86EMUL_EXCEPTION;
+}
+
+static int hvmemul_cmpxchg_gp(
+    unsigned int seg,
+    unsigned long offset,
+    void *old,
+    void *new,
+    unsigned int bytes,
+    struct x86_emulate_ctxt *ctxt)
+{
+    return X86EMUL_EXCEPTION;
+}
+
 static int hvmemul_cmpxchg(
     enum x86_segment seg,
     unsigned long offset,
@@ -1356,6 +1377,13 @@ static int hvmemul_invlpg(
     return rc;
 }
 
+static int hvmemul_vmport_check(
+    unsigned int first_port,
+    struct x86_emulate_ctxt *ctxt)
+{
+    return vmport_check_port(first_port);
+}
+
 static const struct x86_emulate_ops hvm_emulate_ops = {
     .read          = hvmemul_read,
     .insn_fetch    = hvmemul_insn_fetch,
@@ -1379,7 +1407,8 @@ static const struct x86_emulate_ops hvm_emulate_ops = {
     .inject_sw_interrupt = hvmemul_inject_sw_interrupt,
     .get_fpu       = hvmemul_get_fpu,
     .put_fpu       = hvmemul_put_fpu,
-    .invlpg        = hvmemul_invlpg
+    .invlpg        = hvmemul_invlpg,
+    .vmport_check  = hvmemul_vmport_check,
 };
 
 static const struct x86_emulate_ops hvm_emulate_ops_no_write = {
@@ -1405,7 +1434,22 @@ static const struct x86_emulate_ops hvm_emulate_ops_no_write = {
     .inject_sw_interrupt = hvmemul_inject_sw_interrupt,
     .get_fpu       = hvmemul_get_fpu,
     .put_fpu       = hvmemul_put_fpu,
-    .invlpg        = hvmemul_invlpg
+    .invlpg        = hvmemul_invlpg,
+    .vmport_check  = hvmemul_vmport_check,
+};
+
+static const struct x86_emulate_ops hvm_emulate_ops_gp = {
+    .read          = hvmemul_read,
+    .insn_fetch    = hvmemul_insn_fetch,
+    .write         = hvmemul_write_gp,
+    .cmpxchg       = hvmemul_cmpxchg_gp,
+    .read_segment  = hvmemul_read_segment,
+    .write_segment = hvmemul_write_segment,
+    .read_io       = hvmemul_read_io,
+    .write_io      = hvmemul_write_io,
+    .inject_hw_exception = hvmemul_inject_hw_exception,
+    .inject_sw_interrupt = hvmemul_inject_sw_interrupt,
+    .vmport_check  = hvmemul_vmport_check,
 };
 
 static int _hvm_emulate_one(struct hvm_emulate_ctxt *hvmemul_ctxt,
@@ -1522,6 +1566,12 @@ int hvm_emulate_one(
     return _hvm_emulate_one(hvmemul_ctxt, &hvm_emulate_ops);
 }
 
+int hvm_emulate_one_gp(
+    struct hvm_emulate_ctxt *hvmemul_ctxt)
+{
+    return _hvm_emulate_one(hvmemul_ctxt, &hvm_emulate_ops_gp);
+}
+
 int hvm_emulate_one_no_write(
     struct hvm_emulate_ctxt *hvmemul_ctxt)
 {
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 6734fb6..62baf3c 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2119,6 +2119,28 @@ svm_vmexit_do_vmsave(struct vmcb_struct *vmcb,
     return;
 }
 
+static void svm_vmexit_gp_intercept(struct cpu_user_regs *regs,
+                                    struct vcpu *v)
+{
+    struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
+    int rc;
+
+    if ( vmcb->exitinfo1 != 0 || vmcb->exitinfo2 != 0 )
+        rc = X86EMUL_EXCEPTION;
+    else
+    {
+        struct hvm_emulate_ctxt ctxt;
+
+        hvm_emulate_prepare(&ctxt, regs);
+        rc = hvm_emulate_one_gp(&ctxt);
+
+        if ( rc == X86EMUL_OKAY )
+            hvm_emulate_writeback(&ctxt);
+    }
+    if ( rc != X86EMUL_OKAY && rc != X86EMUL_RETRY )
+        hvm_inject_hw_exception(TRAP_gp_fault, vmcb->exitinfo1);
+}
+
 static void svm_vmexit_ud_intercept(struct cpu_user_regs *regs)
 {
     struct hvm_emulate_ctxt ctxt;
@@ -2484,6 +2506,10 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
         break;
     }
 
+    case VMEXIT_EXCEPTION_GP:
+        svm_vmexit_gp_intercept(regs, v);
+        break;
+
     case VMEXIT_EXCEPTION_UD:
         svm_vmexit_ud_intercept(regs);
         break;
diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
index 21292bb..45ead61 100644
--- a/xen/arch/x86/hvm/svm/vmcb.c
+++ b/xen/arch/x86/hvm/svm/vmcb.c
@@ -195,6 +195,8 @@ static int construct_vmcb(struct vcpu *v)
         HVM_TRAP_MASK
         | (1U << TRAP_no_device);
 
+    if ( v->domain->arch.hvm_domain.is_vmware_port_enabled )
+        vmcb->_exception_intercepts |= 1U << TRAP_gp_fault;
     if ( paging_mode_hap(v->domain) )
     {
         vmcb->_np_enable = 1; /* enable nested paging */
diff --git a/xen/arch/x86/hvm/vmware/vmport.c b/xen/arch/x86/hvm/vmware/vmport.c
index 08ddef9..995031c 100644
--- a/xen/arch/x86/hvm/vmware/vmport.c
+++ b/xen/arch/x86/hvm/vmware/vmport.c
@@ -132,6 +132,17 @@ void vmport_register(struct domain *d)
     register_portio_handler(d, BDOOR_PORT, 4, vmport_ioport);
 }
 
+int vmport_check_port(unsigned int port)
+{
+    struct vcpu *curr = current;
+    struct domain *currd = curr->domain;
+
+    if ( port == BDOOR_PORT && is_hvm_domain(currd) &&
+         currd->arch.hvm_domain.is_vmware_port_enabled )
+        return 0;
+    return 1;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 3123706..3f8dbee 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -1151,6 +1151,8 @@ static int construct_vmcs(struct vcpu *v)
 
     v->arch.hvm_vmx.exception_bitmap = HVM_TRAP_MASK
               | (paging_mode_hap(d) ? 0 : (1U << TRAP_page_fault))
+              | (v->domain->arch.hvm_domain.is_vmware_port_enabled ?
+                 (1U << TRAP_gp_fault) : 0)
               | (1U << TRAP_no_device);
     vmx_update_exception_bitmap(v);
 
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 74f563f..fe88afe 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1312,6 +1312,8 @@ static void vmx_update_guest_cr(struct vcpu *v, unsigned int cr)
                 v->arch.hvm_vmx.exception_bitmap = HVM_TRAP_MASK
                           | (paging_mode_hap(v->domain) ?
                              0 : (1U << TRAP_page_fault))
+                          | (v->domain->arch.hvm_domain.is_vmware_port_enabled ?
+                             (1U << TRAP_gp_fault) : 0)
                           | (1U << TRAP_no_device);
                 vmx_update_exception_bitmap(v);
                 vmx_update_debug_state(v);
@@ -2671,6 +2673,38 @@ static void vmx_idtv_reinject(unsigned long idtv_info)
     }
 }
 
+static void vmx_vmexit_gp_intercept(struct cpu_user_regs *regs,
+                                    struct vcpu *v)
+{
+    unsigned long exit_qualification;
+    unsigned long ecode;
+    int rc;
+    unsigned long vector;
+
+    __vmread(VM_EXIT_INTR_INFO, &vector);
+    ASSERT(vector & INTR_INFO_VALID_MASK);
+    ASSERT(vector & INTR_INFO_DELIVER_CODE_MASK);
+
+    __vmread(EXIT_QUALIFICATION, &exit_qualification);
+    __vmread(VM_EXIT_INTR_ERROR_CODE, &ecode);
+
+    if ( ecode != 0 || exit_qualification != 0 )
+        rc = X86EMUL_EXCEPTION;
+    else
+    {
+        struct hvm_emulate_ctxt ctxt;
+
+        hvm_emulate_prepare(&ctxt, regs);
+        rc = hvm_emulate_one_gp(&ctxt);
+
+        if ( rc == X86EMUL_OKAY )
+            hvm_emulate_writeback(&ctxt);
+    }
+
+    if ( rc != X86EMUL_OKAY && rc != X86EMUL_RETRY )
+        hvm_inject_hw_exception(TRAP_gp_fault, ecode);
+}
+
 static int vmx_handle_apic_write(void)
 {
     unsigned long exit_qualification;
@@ -2895,6 +2929,9 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
             HVMTRACE_1D(TRAP, vector);
             vmx_fpu_dirty_intercept();
             break;
+        case TRAP_gp_fault:
+            vmx_vmexit_gp_intercept(regs, v);
+            break;
         case TRAP_page_fault:
             __vmread(EXIT_QUALIFICATION, &exit_qualification);
             __vmread(VM_EXIT_INTR_ERROR_CODE, &ecode);
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index 6c6c58a..602982b 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -3384,8 +3384,11 @@ x86_emulate(
         unsigned int port = ((b < 0xe8)
                              ? insn_fetch_type(uint8_t)
                              : (uint16_t)_regs.edx);
+	bool_t vmport = (ops->vmport_check && /* Vmware backdoor? */
+			 (ops->vmport_check(port, ctxt) == 0));
         op_bytes = !(b & 1) ? 1 : (op_bytes == 8) ? 4 : op_bytes;
-        if ( (rc = ioport_access_check(port, op_bytes, ctxt, ops)) != 0 )
+        if ( !vmport &&
+	     (rc = ioport_access_check(port, op_bytes, ctxt, ops)) != 0 )
             goto done;
         if ( b & 2 )
         {
@@ -3404,6 +3407,14 @@ x86_emulate(
         }
         if ( rc != 0 )
             goto done;
+	if ( vmport )
+	{
+            _regs._ebx = ctxt->regs->_ebx;
+            _regs._ecx = ctxt->regs->_ecx;
+            _regs._edx = ctxt->regs->_edx;
+            _regs._esi = ctxt->regs->_esi;
+            _regs._edi = ctxt->regs->_edi;
+	}
         break;
     }
 
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h b/xen/arch/x86/x86_emulate/x86_emulate.h
index 593b31e..2a866e1 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.h
+++ b/xen/arch/x86/x86_emulate/x86_emulate.h
@@ -393,6 +393,11 @@ struct x86_emulate_ops
         enum x86_segment seg,
         unsigned long offset,
         struct x86_emulate_ctxt *ctxt);
+
+    /* vmport_check */
+    int (*vmport_check)(
+        unsigned int port,
+        struct x86_emulate_ctxt *ctxt);
 };
 
 struct cpu_user_regs;
diff --git a/xen/include/asm-x86/hvm/emulate.h b/xen/include/asm-x86/hvm/emulate.h
index b3971c8..4386169 100644
--- a/xen/include/asm-x86/hvm/emulate.h
+++ b/xen/include/asm-x86/hvm/emulate.h
@@ -36,6 +36,8 @@ struct hvm_emulate_ctxt {
 
 int hvm_emulate_one(
     struct hvm_emulate_ctxt *hvmemul_ctxt);
+int hvm_emulate_one_gp(
+    struct hvm_emulate_ctxt *hvmemul_ctxt);
 int hvm_emulate_one_no_write(
     struct hvm_emulate_ctxt *hvmemul_ctxt);
 void hvm_mem_access_emulate_one(bool_t nowrite,
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index e76f612..c42f7d8 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -523,6 +523,7 @@ extern bool_t opt_hvm_fep;
 #endif
 
 void vmport_register(struct domain *d);
+int vmport_check_port(unsigned int port);
 
 #endif /* __ASM_X86_HVM_HVM_H__ */
 
-- 
1.8.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v10 07/10] tools: Add vmware_port support
  2015-05-14 23:34 [PATCH v10 00/10] Xen VMware tools support Don Slutz
                   ` (5 preceding siblings ...)
  2015-05-14 23:34 ` [PATCH v10 06/10] xen: Add ring 3 " Don Slutz
@ 2015-05-14 23:34 ` Don Slutz
  2015-05-14 23:34 ` [PATCH v10 08/10] Add IOREQ_TYPE_VMWARE_PORT Don Slutz
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Don Slutz @ 2015-05-14 23:34 UTC (permalink / raw)
  To: xen-devel
  Cc: Kevin Tian, Keir Fraser, Ian Campbell, Stefano Stabellini,
	Jun Nakajima, Eddie Dong, Ian Jackson, Don Slutz, Tim Deegan,
	George Dunlap, Aravind Gopalakrishnan, Jan Beulich,
	Andrew Cooper, Boris Ostrovsky, Suravee Suthikulpanit

This new libxl_domain_create_info field is used to set
XEN_DOMCTL_CONFIG_VMWARE_PORT_MASK in the xc_domain_configuration_t
for x86.

In xen it is is_vmware_port_enabled.

If is_vmware_port_enabled then
  enable a limited support of VMware's hyper-call.

VMware's hyper-call is also known as VMware Backdoor I/O Port.

if vmware_port is not specified in the config file, let
"vmware_hwver != 0" be the default value.  This means that only
vmware_hwver = 7 needs to be specified to enable both features.

vmware_hwver = 7 is special because that is what controls the
enable of CPUID leaves for VMware (vmware_hwver >= 7).

Note: vmware_port and nestedhvm cannot be specified at the
same time.

Signed-off-by: Don Slutz <dslutz@verizon.com>
---
v10:
    If..." at the start of the sentence ...
    Also, why is 7 special?


 docs/man/xl.cfg.pod.5       | 16 +++++++++++++++-
 tools/libxl/libxl.h         |  5 +++++
 tools/libxl/libxl_create.c  |  9 +++++++++
 tools/libxl/libxl_types.idl |  1 +
 tools/libxl/libxl_x86.c     |  2 ++
 tools/libxl/xl_cmdimpl.c    |  1 +
 6 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index f62d9f2..3bd0643 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1339,7 +1339,8 @@ Turns on or off the exposure of VMware cpuid.  The number is
 VMware's hardware version number, where 0 is off.  A number >= 7
 is needed to enable exposure of VMware cpuid.
 
-If not zero it changes the default VGA to VMware's VGA.
+If not zero it changes the default VGA to VMware's VGA and the
+default for vmware_port to on.
 
 The hardware version number (vmware_hwver) come from VMware config files.
 
@@ -1352,6 +1353,19 @@ For vssd:VirtualSystemType == vmx-07, vmware_hwver = 7.
 
 =back
 
+=item B<vmware_port=BOOLEAN>
+
+Turns on or off the exposure of VMware port.  This is known as
+vmport in QEMU.  Also called VMware Backdoor I/O Port.  Not all
+defined VMware backdoor commands are implemented.  All of the
+ones that Linux kernel uses are defined.
+
+Defaults to enabled if vmware_hwver is non-zero (i.e. enabled)
+otherwise defaults to disabled.
+
+Note: vmware_port and nestedhvm cannot be specified at the
+same time.
+
 =back
 
 =head3 Emulated VGA Graphics Device
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 61d89be..4f4b41c 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -199,6 +199,11 @@
 #define LIBXL_HAVE_LIBXL_VGA_INTERFACE_TYPE_VMWARE 1
 
 /*
+ * libxl_domain_create_info has the vmware_hwver and vmware_port field.
+ */
+#define LIBXL_HAVE_CREATEINFO_VMWARE 1
+
+/*
  * libxl ABI compatibility
  *
  * The only guarantee which libxl makes regarding ABI compatibility
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index b7818bc..e93e0fe 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -39,6 +39,7 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc,
         libxl_defbool_setdefault(&c_info->hap, libxl_defbool_val(c_info->pvh));
     }
 
+    libxl_defbool_setdefault(&c_info->vmware_port, c_info->vmware_hwver != 0);
     libxl_defbool_setdefault(&c_info->run_hotplug_scripts, true);
     libxl_defbool_setdefault(&c_info->driver_domain, false);
 
@@ -912,6 +913,14 @@ static void initiate_domain_create(libxl__egc *egc,
                                    d_config->c_info.vmware_hwver != 0);
     if (ret) goto error_out;
 
+    if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM &&
+        libxl_defbool_val(d_config->b_info.u.hvm.nested_hvm) &&
+        libxl_defbool_val(d_config->c_info.vmware_port)) {
+        LOG(ERROR,
+            "vmware_port and nestedhvm cannot be enabled simultaneously\n");
+        ret = ERROR_INVAL;
+        goto error_out;
+    }
     if (!sched_params_valid(gc, domid, &d_config->b_info.sched_params)) {
         LOG(ERROR, "Invalid scheduling parameters\n");
         ret = ERROR_INVAL;
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 501bb48..9ef6f0a 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -344,6 +344,7 @@ libxl_domain_create_info = Struct("domain_create_info",[
     ("pvh",          libxl_defbool),
     ("driver_domain",libxl_defbool),
     ("vmware_hwver", uint64),
+    ("vmware_port",  libxl_defbool),
     ], dir=DIR_IN)
 
 libxl_domain_restore_params = Struct("domain_restore_params", [
diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
index fd7dafa..404904a 100644
--- a/tools/libxl/libxl_x86.c
+++ b/tools/libxl/libxl_x86.c
@@ -6,6 +6,8 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
                                       xc_domain_configuration_t *xc_config)
 {
     xc_config->vmware_hwver = d_config->c_info.vmware_hwver;
+    if (libxl_defbool_val(d_config->c_info.vmware_port))
+        xc_config->arch_flags |= XEN_DOMCTL_CONFIG_VMWARE_PORT_MASK;
     return 0;
 }
 
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 18ba70f..100efce 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1230,6 +1230,7 @@ static void parse_config_data(const char *config_source,
     }
 
     xlu_cfg_get_defbool(config, "oos", &c_info->oos, 0);
+    xlu_cfg_get_defbool(config, "vmware_port", &c_info->vmware_port, 0);
 
     if (!xlu_cfg_get_string (config, "pool", &buf, 0))
         xlu_cfg_replace_string(config, "pool", &c_info->pool_name, 0);
-- 
1.8.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v10 08/10] Add IOREQ_TYPE_VMWARE_PORT
  2015-05-14 23:34 [PATCH v10 00/10] Xen VMware tools support Don Slutz
                   ` (6 preceding siblings ...)
  2015-05-14 23:34 ` [PATCH v10 07/10] tools: Add " Don Slutz
@ 2015-05-14 23:34 ` Don Slutz
  2015-05-14 23:34 ` [PATCH v10 09/10] Add xentrace to vmware_port Don Slutz
  2015-05-14 23:34 ` [PATCH v10 10/10] test_x86_emulator.c: Add tests for #GP usage Don Slutz
  9 siblings, 0 replies; 22+ messages in thread
From: Don Slutz @ 2015-05-14 23:34 UTC (permalink / raw)
  To: xen-devel
  Cc: Kevin Tian, Keir Fraser, Ian Campbell, Stefano Stabellini,
	Jun Nakajima, Eddie Dong, Ian Jackson, Don Slutz, Tim Deegan,
	George Dunlap, Aravind Gopalakrishnan, Jan Beulich,
	Andrew Cooper, Boris Ostrovsky, Suravee Suthikulpanit

This adds synchronization of the 6 vcpu registers (only 32bits of
them) that vmport.c needs between Xen and QEMU.

This is to avoid a 2nd and 3rd exchange between QEMU and Xen to
fetch and put these 6 vcpu registers used by the code in vmport.c
and vmmouse.c

In the tools, enable usage of QEMU's vmport code.

The currently most useful VMware port support that QEMU has is the
VMware mouse support.  Xorg included a VMware mouse support that
uses absolute mode.  This make using a mouse in X11 much nicer.

Signed-off-by: Don Slutz <dslutz@verizon.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v10:
  These literals should become an enum.
    I don't think the invalidate type is needed.
    Code handling "case X86EMUL_UNHANDLEABLE:" in emulate.c
    is unclear.
    Comment about "special' range of 1" is not clear.


v9:
  New code was presented as an RFC before this.

  Paul Durrant sugested I add support for other IOREQ types
  to HVMOP_map_io_range_to_ioreq_server.
    I have done this.

 tools/libxc/xc_hvm_build_x86.c   |   5 +-
 tools/libxl/libxl_dm.c           |   2 +
 xen/arch/x86/hvm/emulate.c       |  78 ++++++++++++++---
 xen/arch/x86/hvm/hvm.c           | 182 ++++++++++++++++++++++++++++++++++-----
 xen/arch/x86/hvm/io.c            |  16 ++++
 xen/include/asm-x86/hvm/domain.h |   3 +-
 xen/include/asm-x86/hvm/hvm.h    |   1 +
 xen/include/public/hvm/hvm_op.h  |   5 ++
 xen/include/public/hvm/ioreq.h   |  17 ++++
 xen/include/public/hvm/params.h  |   4 +-
 10 files changed, 274 insertions(+), 39 deletions(-)

diff --git a/tools/libxc/xc_hvm_build_x86.c b/tools/libxc/xc_hvm_build_x86.c
index e45ae4a..ffe52eb 100644
--- a/tools/libxc/xc_hvm_build_x86.c
+++ b/tools/libxc/xc_hvm_build_x86.c
@@ -46,7 +46,8 @@
 #define SPECIALPAGE_IOREQ    5
 #define SPECIALPAGE_IDENT_PT 6
 #define SPECIALPAGE_CONSOLE  7
-#define NR_SPECIAL_PAGES     8
+#define SPECIALPAGE_VMPORT_REGS 8
+#define NR_SPECIAL_PAGES     9
 #define special_pfn(x) (0xff000u - NR_SPECIAL_PAGES + (x))
 
 #define NR_IOREQ_SERVER_PAGES 8
@@ -569,6 +570,8 @@ static int setup_guest(xc_interface *xch,
                      special_pfn(SPECIALPAGE_BUFIOREQ));
     xc_hvm_param_set(xch, dom, HVM_PARAM_IOREQ_PFN,
                      special_pfn(SPECIALPAGE_IOREQ));
+    xc_hvm_param_set(xch, dom, HVM_PARAM_VMPORT_REGS_PFN,
+                     special_pfn(SPECIALPAGE_VMPORT_REGS));
     xc_hvm_param_set(xch, dom, HVM_PARAM_CONSOLE_PFN,
                      special_pfn(SPECIALPAGE_CONSOLE));
     xc_hvm_param_set(xch, dom, HVM_PARAM_PAGING_RING_PFN,
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index c04fa0d..e02766a 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -799,6 +799,8 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
                                             machinearg, max_ram_below_4g);
             }
         }
+        if (libxl_defbool_val(c_info->vmware_port))
+            machinearg = GCSPRINTF("%s,vmport=on", machinearg);
         flexarray_append(dm_args, machinearg);
         for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
             flexarray_append(dm_args, b_info->extra_hvm[i]);
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index d5e6468..0a42d18 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -219,27 +219,70 @@ static int hvmemul_do_io(
             vio->io_state = HVMIO_handle_mmio_awaiting_completion;
         break;
     case X86EMUL_UNHANDLEABLE:
-    {
-        struct hvm_ioreq_server *s =
-            hvm_select_ioreq_server(curr->domain, &p);
-
-        /* If there is no suitable backing DM, just ignore accesses */
-        if ( !s )
+        if ( vmport_check_port(p.addr) )
         {
-            hvm_complete_assist_req(&p);
-            rc = X86EMUL_OKAY;
-            vio->io_state = HVMIO_none;
+            struct hvm_ioreq_server *s =
+                hvm_select_ioreq_server(curr->domain, &p);
+
+            /* If there is no suitable backing DM, just ignore accesses */
+            if ( !s )
+            {
+                hvm_complete_assist_req(&p);
+                rc = X86EMUL_OKAY;
+                vio->io_state = HVMIO_none;
+            }
+            else
+            {
+                rc = X86EMUL_RETRY;
+                if ( !hvm_send_assist_req(s, &p) )
+                    vio->io_state = HVMIO_none;
+                else if ( p_data == NULL )
+                    rc = X86EMUL_OKAY;
+            }
         }
         else
         {
-            rc = X86EMUL_RETRY;
-            if ( !hvm_send_assist_req(s, &p) )
-                vio->io_state = HVMIO_none;
-            else if ( p_data == NULL )
+            struct hvm_ioreq_server *s;
+            vmware_regs_t *vr;
+
+            BUILD_BUG_ON(sizeof(ioreq_t) < sizeof(vmware_regs_t));
+
+            p.type = IOREQ_TYPE_VMWARE_PORT;
+            s = hvm_select_ioreq_server(curr->domain, &p);
+            vr = get_vmport_regs_any(s, curr);
+
+            /*
+             * If there is no suitable backing DM, just ignore accesses.  If
+             * we do not have access to registers to pass to QEMU, just
+             * ignore access.
+             */
+            if ( !s || !vr )
+            {
+                hvm_complete_assist_req(&p);
                 rc = X86EMUL_OKAY;
+                vio->io_state = HVMIO_none;
+            }
+            else
+            {
+                struct cpu_user_regs *regs = guest_cpu_user_regs();
+
+                p.data = regs->rax;
+                vr->ebx = regs->_ebx;
+                vr->ecx = regs->_ecx;
+                vr->edx = regs->_edx;
+                vr->esi = regs->_esi;
+                vr->edi = regs->_edi;
+
+                vio->io_state = HVMIO_handle_pio_awaiting_completion;
+                if ( !hvm_send_assist_req(s, &p) )
+                {
+                    rc = X86EMUL_RETRY;
+                    vio->io_state = HVMIO_none;
+                }
+                /* else leave rc as X86EMUL_UNHANDLEABLE for below. */
+            }
         }
         break;
-    }
     default:
         BUG();
     }
@@ -248,6 +291,13 @@ static int hvmemul_do_io(
     {
         if ( ram_page )
             put_page(ram_page);
+        /*
+         * If rc is still X86EMUL_UNHANDLEABLE, then were are of
+         * type IOREQ_TYPE_VMWARE_PORT, so completion in
+         * hvm_io_assist() with no re-emulation required
+         */
+        if ( rc == X86EMUL_UNHANDLEABLE )
+            rc = X86EMUL_OKAY;
         return rc;
     }
 
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index a179123..d2ad6e2 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -390,6 +390,47 @@ static ioreq_t *get_ioreq(struct hvm_ioreq_server *s, struct vcpu *v)
     return &p->vcpu_ioreq[v->vcpu_id];
 }
 
+static vmware_regs_t *get_vmport_regs_one(struct hvm_ioreq_server *s,
+                                          struct vcpu *v)
+{
+    struct hvm_ioreq_vcpu *sv;
+
+    list_for_each_entry ( sv,
+                          &s->ioreq_vcpu_list,
+                          list_entry )
+    {
+        if ( sv->vcpu == v )
+        {
+            shared_vmport_iopage_t *p = s->vmport_ioreq.va;
+            if ( !p )
+                return NULL;
+            return &p->vcpu_vmport_regs[v->vcpu_id];
+        }
+    }
+    return NULL;
+}
+
+vmware_regs_t *get_vmport_regs_any(struct hvm_ioreq_server *s, struct vcpu *v)
+{
+    struct domain *d = v->domain;
+
+    ASSERT((v == current) || !vcpu_runnable(v));
+
+    if ( s )
+        return get_vmport_regs_one(s, v);
+
+    list_for_each_entry ( s,
+                          &d->arch.hvm_domain.ioreq_server.list,
+                          list_entry )
+    {
+        vmware_regs_t *ret = get_vmport_regs_one(s, v);
+
+        if ( ret )
+            return ret;
+    }
+    return NULL;
+}
+
 bool_t hvm_io_pending(struct vcpu *v)
 {
     struct domain *d = v->domain;
@@ -500,22 +541,56 @@ static void hvm_free_ioreq_gmfn(struct domain *d, unsigned long gmfn)
         set_bit(i, &d->arch.hvm_domain.ioreq_gmfn.mask);
 }
 
-static void hvm_unmap_ioreq_page(struct hvm_ioreq_server *s, bool_t buf)
+typedef enum {
+    IOREQ_PAGE_TYPE_IOREQ,
+    IOREQ_PAGE_TYPE_BUFIOREQ,
+    IOREQ_PAGE_TYPE_VMPORT,
+} ioreq_page_type_t;
+
+static void hvm_unmap_ioreq_page(struct hvm_ioreq_server *s, ioreq_page_type_t buf)
 {
-    struct hvm_ioreq_page *iorp = buf ? &s->bufioreq : &s->ioreq;
+    struct hvm_ioreq_page *iorp = NULL;
+
+    switch ( buf )
+    {
+    case IOREQ_PAGE_TYPE_IOREQ:
+        iorp = &s->ioreq;
+        break;
+    case IOREQ_PAGE_TYPE_BUFIOREQ:
+        iorp = &s->bufioreq;
+        break;
+    case IOREQ_PAGE_TYPE_VMPORT:
+        iorp = &s->vmport_ioreq;
+        break;
+    }
+    ASSERT(iorp);
 
     destroy_ring_for_helper(&iorp->va, iorp->page);
 }
 
 static int hvm_map_ioreq_page(
-    struct hvm_ioreq_server *s, bool_t buf, unsigned long gmfn)
+    struct hvm_ioreq_server *s, ioreq_page_type_t buf, unsigned long gmfn)
 {
     struct domain *d = s->domain;
-    struct hvm_ioreq_page *iorp = buf ? &s->bufioreq : &s->ioreq;
+    struct hvm_ioreq_page *iorp = NULL;
     struct page_info *page;
     void *va;
     int rc;
 
+    switch ( buf )
+    {
+    case IOREQ_PAGE_TYPE_IOREQ:
+        iorp = &s->ioreq;
+        break;
+    case IOREQ_PAGE_TYPE_BUFIOREQ:
+        iorp = &s->bufioreq;
+        break;
+    case IOREQ_PAGE_TYPE_VMPORT:
+        iorp = &s->vmport_ioreq;
+        break;
+    }
+    ASSERT(iorp);
+
     if ( (rc = prepare_ring_for_helper(d, gmfn, &page, &va)) )
         return rc;
 
@@ -732,19 +807,32 @@ static void hvm_ioreq_server_remove_all_vcpus(struct hvm_ioreq_server *s)
 
 static int hvm_ioreq_server_map_pages(struct hvm_ioreq_server *s,
                                       unsigned long ioreq_pfn,
-                                      unsigned long bufioreq_pfn)
+                                      unsigned long bufioreq_pfn,
+                                      unsigned long vmport_ioreq_pfn)
 {
     int rc;
 
-    rc = hvm_map_ioreq_page(s, 0, ioreq_pfn);
+    rc = hvm_map_ioreq_page(s, IOREQ_PAGE_TYPE_IOREQ, ioreq_pfn);
     if ( rc )
         return rc;
 
     if ( bufioreq_pfn != INVALID_GFN )
-        rc = hvm_map_ioreq_page(s, 1, bufioreq_pfn);
+        rc = hvm_map_ioreq_page(s, IOREQ_PAGE_TYPE_BUFIOREQ, bufioreq_pfn);
 
     if ( rc )
-        hvm_unmap_ioreq_page(s, 0);
+    {
+        hvm_unmap_ioreq_page(s, IOREQ_PAGE_TYPE_IOREQ);
+        return rc;
+    }
+
+    rc = hvm_map_ioreq_page(s, IOREQ_PAGE_TYPE_VMPORT, vmport_ioreq_pfn);
+
+    if ( rc )
+    {
+        if ( bufioreq_pfn != INVALID_GFN )
+            hvm_unmap_ioreq_page(s, IOREQ_PAGE_TYPE_BUFIOREQ);
+        hvm_unmap_ioreq_page(s, IOREQ_PAGE_TYPE_IOREQ);
+    }
 
     return rc;
 }
@@ -756,6 +844,8 @@ static int hvm_ioreq_server_setup_pages(struct hvm_ioreq_server *s,
     struct domain *d = s->domain;
     unsigned long ioreq_pfn = INVALID_GFN;
     unsigned long bufioreq_pfn = INVALID_GFN;
+    unsigned long vmport_ioreq_pfn =
+        d->arch.hvm_domain.params[HVM_PARAM_VMPORT_REGS_PFN];
     int rc;
 
     if ( is_default )
@@ -767,7 +857,8 @@ static int hvm_ioreq_server_setup_pages(struct hvm_ioreq_server *s,
         ASSERT(handle_bufioreq);
         return hvm_ioreq_server_map_pages(s,
                    d->arch.hvm_domain.params[HVM_PARAM_IOREQ_PFN],
-                   d->arch.hvm_domain.params[HVM_PARAM_BUFIOREQ_PFN]);
+                   d->arch.hvm_domain.params[HVM_PARAM_BUFIOREQ_PFN],
+                   vmport_ioreq_pfn);
     }
 
     rc = hvm_alloc_ioreq_gmfn(d, &ioreq_pfn);
@@ -776,8 +867,8 @@ static int hvm_ioreq_server_setup_pages(struct hvm_ioreq_server *s,
         rc = hvm_alloc_ioreq_gmfn(d, &bufioreq_pfn);
 
     if ( !rc )
-        rc = hvm_ioreq_server_map_pages(s, ioreq_pfn, bufioreq_pfn);
-
+        rc = hvm_ioreq_server_map_pages(s, ioreq_pfn, bufioreq_pfn,
+                                        vmport_ioreq_pfn);
     if ( rc )
     {
         hvm_free_ioreq_gmfn(d, ioreq_pfn);
@@ -792,11 +883,15 @@ static void hvm_ioreq_server_unmap_pages(struct hvm_ioreq_server *s,
 {
     struct domain *d = s->domain;
     bool_t handle_bufioreq = ( s->bufioreq.va != NULL );
+    bool_t handle_vmport_ioreq = ( s->vmport_ioreq.va != NULL );
+
+    if ( handle_vmport_ioreq )
+        hvm_unmap_ioreq_page(s, IOREQ_PAGE_TYPE_VMPORT);
 
     if ( handle_bufioreq )
-        hvm_unmap_ioreq_page(s, 1);
+        hvm_unmap_ioreq_page(s, IOREQ_PAGE_TYPE_BUFIOREQ);
 
-    hvm_unmap_ioreq_page(s, 0);
+    hvm_unmap_ioreq_page(s, IOREQ_PAGE_TYPE_IOREQ);
 
     if ( !is_default )
     {
@@ -831,12 +926,38 @@ static int hvm_ioreq_server_alloc_rangesets(struct hvm_ioreq_server *s,
     for ( i = 0; i < NR_IO_RANGE_TYPES; i++ )
     {
         char *name;
+        char *type_name = NULL;
+        unsigned int limit;
 
-        rc = asprintf(&name, "ioreq_server %d %s", s->id,
-                      (i == HVMOP_IO_RANGE_PORT) ? "port" :
-                      (i == HVMOP_IO_RANGE_MEMORY) ? "memory" :
-                      (i == HVMOP_IO_RANGE_PCI) ? "pci" :
-                      "");
+        switch ( i )
+        {
+        case HVMOP_IO_RANGE_PORT:
+            type_name = "port";
+            limit = MAX_NR_IO_RANGES;
+            break;
+        case HVMOP_IO_RANGE_MEMORY:
+            type_name = "memory";
+            limit = MAX_NR_IO_RANGES;
+            break;
+        case HVMOP_IO_RANGE_PCI:
+            type_name = "pci";
+            limit = MAX_NR_IO_RANGES;
+            break;
+        case HVMOP_IO_RANGE_VMWARE_PORT:
+            type_name = "VMware port";
+            limit = 1;
+            break;
+        case HVMOP_IO_RANGE_TIMEOFFSET:
+            type_name = "timeoffset";
+            limit = 1;
+            break;
+        default:
+            break;
+        }
+        if ( !type_name )
+            continue;
+
+        rc = asprintf(&name, "ioreq_server %d %s", s->id, type_name);
         if ( rc )
             goto fail;
 
@@ -849,7 +970,12 @@ static int hvm_ioreq_server_alloc_rangesets(struct hvm_ioreq_server *s,
         if ( !s->range[i] )
             goto fail;
 
-        rangeset_limit(s->range[i], MAX_NR_IO_RANGES);
+        rangeset_limit(s->range[i], limit);
+
+        /* VMware port */
+        if ( i == HVMOP_IO_RANGE_VMWARE_PORT &&
+            s->domain->arch.hvm_domain.is_vmware_port_enabled )
+            rc = rangeset_add_range(s->range[i], 1, 1);
     }
 
  done:
@@ -1147,6 +1273,8 @@ static int hvm_map_io_range_to_ioreq_server(struct domain *d, ioservid_t id,
             case HVMOP_IO_RANGE_PORT:
             case HVMOP_IO_RANGE_MEMORY:
             case HVMOP_IO_RANGE_PCI:
+            case HVMOP_IO_RANGE_VMWARE_PORT:
+            case HVMOP_IO_RANGE_TIMEOFFSET:
                 r = s->range[type];
                 break;
 
@@ -1198,6 +1326,8 @@ static int hvm_unmap_io_range_from_ioreq_server(struct domain *d, ioservid_t id,
             case HVMOP_IO_RANGE_PORT:
             case HVMOP_IO_RANGE_MEMORY:
             case HVMOP_IO_RANGE_PCI:
+            case HVMOP_IO_RANGE_VMWARE_PORT:
+            case HVMOP_IO_RANGE_TIMEOFFSET:
                 r = s->range[type];
                 break;
 
@@ -2406,9 +2536,6 @@ struct hvm_ioreq_server *hvm_select_ioreq_server(struct domain *d,
     if ( list_empty(&d->arch.hvm_domain.ioreq_server.list) )
         return NULL;
 
-    if ( p->type != IOREQ_TYPE_COPY && p->type != IOREQ_TYPE_PIO )
-        return d->arch.hvm_domain.default_ioreq_server;
-
     cf8 = d->arch.hvm_domain.pci_cf8;
 
     if ( p->type == IOREQ_TYPE_PIO &&
@@ -2451,7 +2578,10 @@ struct hvm_ioreq_server *hvm_select_ioreq_server(struct domain *d,
         BUILD_BUG_ON(IOREQ_TYPE_PIO != HVMOP_IO_RANGE_PORT);
         BUILD_BUG_ON(IOREQ_TYPE_COPY != HVMOP_IO_RANGE_MEMORY);
         BUILD_BUG_ON(IOREQ_TYPE_PCI_CONFIG != HVMOP_IO_RANGE_PCI);
+        BUILD_BUG_ON(IOREQ_TYPE_VMWARE_PORT != HVMOP_IO_RANGE_VMWARE_PORT);
+        BUILD_BUG_ON(IOREQ_TYPE_TIMEOFFSET != HVMOP_IO_RANGE_TIMEOFFSET);
         r = s->range[type];
+        ASSERT(r);
 
         switch ( type )
         {
@@ -2478,6 +2608,13 @@ struct hvm_ioreq_server *hvm_select_ioreq_server(struct domain *d,
             }
 
             break;
+        case IOREQ_TYPE_VMWARE_PORT:
+        case IOREQ_TYPE_TIMEOFFSET:
+            /* The 'special' range of [1,1] is checked for being enabled */
+            if ( rangeset_contains_singleton(r, 1) )
+                return s;
+
+            break;
         }
     }
 
@@ -2637,6 +2774,7 @@ void hvm_complete_assist_req(ioreq_t *p)
     case IOREQ_TYPE_PCI_CONFIG:
         ASSERT_UNREACHABLE();
         break;
+    case IOREQ_TYPE_VMWARE_PORT:
     case IOREQ_TYPE_COPY:
     case IOREQ_TYPE_PIO:
         if ( p->dir == IOREQ_READ )
diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
index 68fb890..7684cf0 100644
--- a/xen/arch/x86/hvm/io.c
+++ b/xen/arch/x86/hvm/io.c
@@ -192,6 +192,22 @@ void hvm_io_assist(ioreq_t *p)
         (void)handle_mmio();
         break;
     case HVMIO_handle_pio_awaiting_completion:
+        if ( p->type == IOREQ_TYPE_VMWARE_PORT )
+        {
+            vmware_regs_t *vr = get_vmport_regs_any(NULL, curr);
+
+            if ( vr )
+            {
+                struct cpu_user_regs *regs = guest_cpu_user_regs();
+
+                /* Only change the 32bit part of the register */
+                regs->_ebx = vr->ebx;
+                regs->_ecx = vr->ecx;
+                regs->_edx = vr->edx;
+                regs->_esi = vr->esi;
+                regs->_edi = vr->edi;
+            }
+        }
         if ( vio->io_size == 4 ) /* Needs zero extension. */
             guest_cpu_user_regs()->rax = (uint32_t)p->data;
         else
diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
index d3166e4..6c47e6d 100644
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -48,7 +48,7 @@ struct hvm_ioreq_vcpu {
     evtchn_port_t    ioreq_evtchn;
 };
 
-#define NR_IO_RANGE_TYPES (HVMOP_IO_RANGE_PCI + 1)
+#define NR_IO_RANGE_TYPES (HVMOP_IO_RANGE_VMWARE_PORT + 1)
 #define MAX_NR_IO_RANGES  256
 
 struct hvm_ioreq_server {
@@ -63,6 +63,7 @@ struct hvm_ioreq_server {
     ioservid_t             id;
     struct hvm_ioreq_page  ioreq;
     struct list_head       ioreq_vcpu_list;
+    struct hvm_ioreq_page  vmport_ioreq;
     struct hvm_ioreq_page  bufioreq;
 
     /* Lock to serialize access to buffered ioreq ring */
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index c42f7d8..0c72ac8 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -524,6 +524,7 @@ extern bool_t opt_hvm_fep;
 
 void vmport_register(struct domain *d);
 int vmport_check_port(unsigned int port);
+vmware_regs_t *get_vmport_regs_any(struct hvm_ioreq_server *s, struct vcpu *v);
 
 #endif /* __ASM_X86_HVM_HVM_H__ */
 
diff --git a/xen/include/public/hvm/hvm_op.h b/xen/include/public/hvm/hvm_op.h
index cde3571..2dcafc3 100644
--- a/xen/include/public/hvm/hvm_op.h
+++ b/xen/include/public/hvm/hvm_op.h
@@ -314,6 +314,9 @@ DEFINE_XEN_GUEST_HANDLE(xen_hvm_get_ioreq_server_info_t);
  *
  * NOTE: unless an emulation request falls entirely within a range mapped
  * by a secondary emulator, it will not be passed to that emulator.
+ *
+ * NOTE: The 'special' range of [1,1] is what is checked for on
+ * TIMEOFFSET and VMWARE_PORT.
  */
 #define HVMOP_map_io_range_to_ioreq_server 19
 #define HVMOP_unmap_io_range_from_ioreq_server 20
@@ -324,6 +327,8 @@ struct xen_hvm_io_range {
 # define HVMOP_IO_RANGE_PORT   0 /* I/O port range */
 # define HVMOP_IO_RANGE_MEMORY 1 /* MMIO range */
 # define HVMOP_IO_RANGE_PCI    2 /* PCI segment/bus/dev/func range */
+# define HVMOP_IO_RANGE_TIMEOFFSET 7 /* TIMEOFFSET special range */
+# define HVMOP_IO_RANGE_VMWARE_PORT 9 /* VMware port special range */
     uint64_aligned_t start, end; /* IN - inclusive start and end of range */
 };
 typedef struct xen_hvm_io_range xen_hvm_io_range_t;
diff --git a/xen/include/public/hvm/ioreq.h b/xen/include/public/hvm/ioreq.h
index 5b5fedf..2d9dcbe 100644
--- a/xen/include/public/hvm/ioreq.h
+++ b/xen/include/public/hvm/ioreq.h
@@ -37,6 +37,7 @@
 #define IOREQ_TYPE_PCI_CONFIG   2
 #define IOREQ_TYPE_TIMEOFFSET   7
 #define IOREQ_TYPE_INVALIDATE   8 /* mapcache */
+#define IOREQ_TYPE_VMWARE_PORT  9 /* pio + vmport registers */
 
 /*
  * VMExit dispatcher should cooperate with instruction decoder to
@@ -48,6 +49,8 @@
  * 
  * 63....48|47..40|39..35|34..32|31........0
  * SEGMENT |BUS   |DEV   |FN    |OFFSET
+ *
+ * For I/O type IOREQ_TYPE_VMWARE_PORT also use the vmware_regs.
  */
 struct ioreq {
     uint64_t addr;          /* physical address */
@@ -66,11 +69,25 @@ struct ioreq {
 };
 typedef struct ioreq ioreq_t;
 
+struct vmware_regs {
+    uint32_t esi;
+    uint32_t edi;
+    uint32_t ebx;
+    uint32_t ecx;
+    uint32_t edx;
+};
+typedef struct vmware_regs vmware_regs_t;
+
 struct shared_iopage {
     struct ioreq vcpu_ioreq[1];
 };
 typedef struct shared_iopage shared_iopage_t;
 
+struct shared_vmport_iopage {
+    struct vmware_regs vcpu_vmport_regs[1];
+};
+typedef struct shared_vmport_iopage shared_vmport_iopage_t;
+
 struct buf_ioreq {
     uint8_t  type;   /* I/O type                    */
     uint8_t  pad:1;
diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
index 7c73089..130eba9 100644
--- a/xen/include/public/hvm/params.h
+++ b/xen/include/public/hvm/params.h
@@ -50,6 +50,8 @@
 #define HVM_PARAM_PAE_ENABLED  4
 
 #define HVM_PARAM_IOREQ_PFN    5
+/* Extra vmport PFN. */
+#define HVM_PARAM_VMPORT_REGS_PFN 35
 
 #define HVM_PARAM_BUFIOREQ_PFN 6
 #define HVM_PARAM_BUFIOREQ_EVTCHN 26
@@ -187,6 +189,6 @@
 /* Location of the VM Generation ID in guest physical address space. */
 #define HVM_PARAM_VM_GENERATION_ID_ADDR 34
 
-#define HVM_NR_PARAMS          35
+#define HVM_NR_PARAMS          36
 
 #endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */
-- 
1.8.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v10 09/10] Add xentrace to vmware_port
  2015-05-14 23:34 [PATCH v10 00/10] Xen VMware tools support Don Slutz
                   ` (7 preceding siblings ...)
  2015-05-14 23:34 ` [PATCH v10 08/10] Add IOREQ_TYPE_VMWARE_PORT Don Slutz
@ 2015-05-14 23:34 ` Don Slutz
  2015-05-14 23:34 ` [PATCH v10 10/10] test_x86_emulator.c: Add tests for #GP usage Don Slutz
  9 siblings, 0 replies; 22+ messages in thread
From: Don Slutz @ 2015-05-14 23:34 UTC (permalink / raw)
  To: xen-devel
  Cc: Kevin Tian, Keir Fraser, Ian Campbell, Stefano Stabellini,
	Jun Nakajima, Eddie Dong, Ian Jackson, Don Slutz, Tim Deegan,
	George Dunlap, Aravind Gopalakrishnan, Jan Beulich,
	Andrew Cooper, Boris Ostrovsky, Suravee Suthikulpanit

Also added missing TRAP_DEBUG & VLAPIC.

Signed-off-by: Don Slutz <dslutz@verizon.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v10:
  Added Acked-by: Ian Campbell
  Added back in the trace point calls.

    Why is cmd in this patch?
      Because the trace points use it.

v9:
  Dropped unneed VMPORT_UNHANDLED, VMPORT_DECODE.

v7:
      Dropped some of the new traces.
      Added HVMTRACE_ND7.

v6:
      Dropped the attempt to use svm_nextrip_insn_length via
      __get_instruction_length (added in v2).  Just always look
      at upto 15 bytes on AMD.

v5:
      exitinfo1 is used twice.
        Fixed.

 tools/xentrace/formats           |  5 +++++
 xen/arch/x86/hvm/io.c            |  3 +++
 xen/arch/x86/hvm/vmware/vmport.c | 17 ++++++++++++++---
 xen/include/asm-x86/hvm/trace.h  | 22 ++++++++++++++++++++++
 xen/include/public/trace.h       |  3 +++
 5 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/tools/xentrace/formats b/tools/xentrace/formats
index 5d7b72a..eec65f4 100644
--- a/tools/xentrace/formats
+++ b/tools/xentrace/formats
@@ -79,6 +79,11 @@
 0x00082020  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  INTR_WINDOW [ value = 0x%(1)08x ]
 0x00082021  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  NPF         [ gpa = 0x%(2)08x%(1)08x mfn = 0x%(4)08x%(3)08x qual = 0x%(5)04x p2mt = 0x%(6)04x ]
 0x00082023  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  TRAP        [ vector = 0x%(1)02x ]
+0x00082024  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  TRAP_DEBUG  [ exit_qualification = 0x%(1)08x ]
+0x00082025  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  VLAPIC
+0x00082026  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  VMPORT_HANDLED   [ cmd = %(1)d eax = 0x%(2)08x ebx = 0x%(3)08x ecx = 0x%(4)08x edx = 0x%(5)08x esi = 0x%(6)08x edi = 0x%(7)08x ]
+0x00082027  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  VMPORT_IGNORED   [ port = %(1)d eax = 0x%(2)08x ebx = 0x%(3)08x ecx = 0x%(4)08x edx = 0x%(5)08x esi = 0x%(6)08x edi = 0x%(7)08x ]
+0x00082028  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  VMPORT_QEMU      [ eax = 0x%(1)08x ebx = 0x%(2)08x ecx = 0x%(3)08x edx = 0x%(4)08x esi = 0x%(5)08x edi = 0x%(6)08x ]
 
 0x0010f001  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  page_grant_map      [ domid = %(1)d ]
 0x0010f002  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  page_grant_unmap    [ domid = %(1)d ]
diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
index 7684cf0..6a9cfb0 100644
--- a/xen/arch/x86/hvm/io.c
+++ b/xen/arch/x86/hvm/io.c
@@ -206,6 +206,9 @@ void hvm_io_assist(ioreq_t *p)
                 regs->_edx = vr->edx;
                 regs->_esi = vr->esi;
                 regs->_edi = vr->edi;
+                HVMTRACE_ND(VMPORT_QEMU, 0, 1/*cycles*/, 6,
+                            p->data, regs->_ebx, regs->_ecx,
+                            regs->_edx, regs->_esi, regs->_edi);
             }
         }
         if ( vio->io_size == 4 ) /* Needs zero extension. */
diff --git a/xen/arch/x86/hvm/vmware/vmport.c b/xen/arch/x86/hvm/vmware/vmport.c
index 995031c..408e14f 100644
--- a/xen/arch/x86/hvm/vmware/vmport.c
+++ b/xen/arch/x86/hvm/vmware/vmport.c
@@ -16,6 +16,7 @@
 #include <xen/lib.h>
 #include <asm/hvm/hvm.h>
 #include <asm/hvm/support.h>
+#include <asm/hvm/trace.h>
 
 #include "backdoor_def.h"
 
@@ -35,6 +36,7 @@ static int vmport_ioport(int dir, uint32_t port, uint32_t bytes, uint32_t *val)
     if ( port == BDOOR_PORT && regs->_eax == BDOOR_MAGIC )
     {
         uint32_t new_eax = ~0u;
+        uint16_t cmd = regs->_ecx;
         uint64_t value;
         struct vcpu *curr = current;
         struct domain *currd = curr->domain;
@@ -46,7 +48,7 @@ static int vmport_ioport(int dir, uint32_t port, uint32_t bytes, uint32_t *val)
          * leaving the high 32-bits unchanged, unlike what one would
          * expect to happen.
          */
-        switch ( regs->_ecx & 0xffff )
+        switch ( cmd )
         {
         case BDOOR_CMD_GETMHZ:
             new_eax = currd->arch.tsc_khz / 1000;
@@ -118,11 +120,20 @@ static int vmport_ioport(int dir, uint32_t port, uint32_t bytes, uint32_t *val)
             /* Let backing DM handle */
             return X86EMUL_UNHANDLEABLE;
         }
+        HVMTRACE_ND7(VMPORT_HANDLED, 0, 0/*cycles*/, 7,
+                     cmd, new_eax, regs->_ebx, regs->_ecx,
+                     regs->_edx, regs->_esi, regs->_edi);
         if ( dir == IOREQ_READ )
             *val = new_eax;
     }
-    else if ( dir == IOREQ_READ )
-        *val = ~0u;
+    else
+    {
+        HVMTRACE_ND7(VMPORT_IGNORED, 0, 0/*cycles*/, 7,
+                     port, regs->_eax, regs->_ebx, regs->_ecx,
+                     regs->_edx, regs->_esi, regs->_edi);
+        if ( dir == IOREQ_READ )
+            *val = ~0u;
+    }
 
     return X86EMUL_OKAY;
 }
diff --git a/xen/include/asm-x86/hvm/trace.h b/xen/include/asm-x86/hvm/trace.h
index de802a6..0ad805f 100644
--- a/xen/include/asm-x86/hvm/trace.h
+++ b/xen/include/asm-x86/hvm/trace.h
@@ -54,6 +54,9 @@
 #define DO_TRC_HVM_TRAP             DEFAULT_HVM_MISC
 #define DO_TRC_HVM_TRAP_DEBUG       DEFAULT_HVM_MISC
 #define DO_TRC_HVM_VLAPIC           DEFAULT_HVM_MISC
+#define DO_TRC_HVM_VMPORT_HANDLED   DEFAULT_HVM_IO
+#define DO_TRC_HVM_VMPORT_IGNORED   DEFAULT_HVM_IO
+#define DO_TRC_HVM_VMPORT_QEMU      DEFAULT_HVM_IO
 
 
 #define TRC_PAR_LONG(par) ((par)&0xFFFFFFFF),((par)>>32)
@@ -83,6 +86,25 @@
         }                                                                 \
     } while(0)
 
+#define HVMTRACE_ND7(evt, modifier, cycles, count, d1, d2, d3, d4, d5, d6, d7) \
+    do {                                                                  \
+        if ( unlikely(tb_init_done) && DO_TRC_HVM_ ## evt )               \
+        {                                                                 \
+            struct {                                                      \
+                u32 d[7];                                                 \
+            } _d;                                                         \
+            _d.d[0]=(d1);                                                 \
+            _d.d[1]=(d2);                                                 \
+            _d.d[2]=(d3);                                                 \
+            _d.d[3]=(d4);                                                 \
+            _d.d[4]=(d5);                                                 \
+            _d.d[5]=(d6);                                                 \
+            _d.d[6]=(d7);                                                 \
+            __trace_var(TRC_HVM_ ## evt | (modifier), cycles,             \
+                        sizeof(*_d.d) * count, &_d);                      \
+        }                                                                 \
+    } while(0)
+
 #define HVMTRACE_6D(evt, d1, d2, d3, d4, d5, d6)    \
     HVMTRACE_ND(evt, 0, 0, 6, d1, d2, d3, d4, d5, d6)
 #define HVMTRACE_5D(evt, d1, d2, d3, d4, d5)        \
diff --git a/xen/include/public/trace.h b/xen/include/public/trace.h
index 5211ae7..16b87f9 100644
--- a/xen/include/public/trace.h
+++ b/xen/include/public/trace.h
@@ -227,6 +227,9 @@
 #define TRC_HVM_TRAP             (TRC_HVM_HANDLER + 0x23)
 #define TRC_HVM_TRAP_DEBUG       (TRC_HVM_HANDLER + 0x24)
 #define TRC_HVM_VLAPIC           (TRC_HVM_HANDLER + 0x25)
+#define TRC_HVM_VMPORT_HANDLED   (TRC_HVM_HANDLER + 0x26)
+#define TRC_HVM_VMPORT_IGNORED   (TRC_HVM_HANDLER + 0x27)
+#define TRC_HVM_VMPORT_QEMU      (TRC_HVM_HANDLER + 0x28)
 
 #define TRC_HVM_IOPORT_WRITE    (TRC_HVM_HANDLER + 0x216)
 #define TRC_HVM_IOMEM_WRITE     (TRC_HVM_HANDLER + 0x217)
-- 
1.8.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v10 10/10] test_x86_emulator.c: Add tests for #GP usage
  2015-05-14 23:34 [PATCH v10 00/10] Xen VMware tools support Don Slutz
                   ` (8 preceding siblings ...)
  2015-05-14 23:34 ` [PATCH v10 09/10] Add xentrace to vmware_port Don Slutz
@ 2015-05-14 23:34 ` Don Slutz
  9 siblings, 0 replies; 22+ messages in thread
From: Don Slutz @ 2015-05-14 23:34 UTC (permalink / raw)
  To: xen-devel
  Cc: Kevin Tian, Keir Fraser, Ian Campbell, Stefano Stabellini,
	Jun Nakajima, Eddie Dong, Ian Jackson, Don Slutz, Tim Deegan,
	George Dunlap, Aravind Gopalakrishnan, Jan Beulich,
	Andrew Cooper, Boris Ostrovsky, Suravee Suthikulpanit

Test out special #GP handling for the VMware port 0x5658.
This is done in two "modes", j=0 and j=1.  Testing 4
instructions (all the basic PIO) in both modes.

The port used is based on j.

For IN, eax should change.  For OUT eax should not change.

All 4 PIO instructions are 1 byte long, so eip should only
change by 1.

Signed-off-by: Don Slutz <dslutz@verizon.com>
---
v10:
  More comments and simpler error checking.
     Dropped un-needed new routines.

 tools/tests/x86_emulator/test_x86_emulator.c | 189 +++++++++++++++++++++++++++
 1 file changed, 189 insertions(+)

diff --git a/tools/tests/x86_emulator/test_x86_emulator.c b/tools/tests/x86_emulator/test_x86_emulator.c
index 1b78bf7..a509dad 100644
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -154,6 +154,47 @@ int get_fpu(
     return X86EMUL_OKAY;
 }
 
+static int read_io(
+    unsigned int port,
+    unsigned int bytes,
+    unsigned long *val,
+    struct x86_emulate_ctxt *ctxt)
+{
+    *val = 0xffffffff;
+    if ( port == 0x5658 )
+    {
+        ctxt->regs->_ebx++;
+        ctxt->regs->_ecx++;
+        ctxt->regs->_esi++;
+    }
+    return X86EMUL_OKAY;
+}
+
+static int write_io(
+    unsigned int port,
+    unsigned int bytes,
+    unsigned long val,
+    struct x86_emulate_ctxt *ctxt)
+{
+    if ( port == 0x5658 )
+    {
+        ctxt->regs->_ebx++;
+        ctxt->regs->_ecx++;
+        ctxt->regs->_esi++;
+    }
+    return X86EMUL_OKAY;
+}
+
+static int vmport_check(
+	unsigned int first_port,
+	struct x86_emulate_ctxt *ctxt)
+{
+    if ( first_port == 0x5658 )
+        return 0;
+    else
+        return 1;
+}
+
 static struct x86_emulate_ops emulops = {
     .read       = read,
     .insn_fetch = fetch,
@@ -163,6 +204,13 @@ static struct x86_emulate_ops emulops = {
     .get_fpu    = get_fpu,
 };
 
+static struct x86_emulate_ops emulops_gp = {
+    .insn_fetch = fetch,
+    .read_io    = read_io,
+    .write_io   = write_io,
+    .vmport_check = vmport_check,
+};
+
 int main(int argc, char **argv)
 {
     struct x86_emulate_ctxt ctxt;
@@ -928,6 +976,147 @@ int main(int argc, char **argv)
         goto fail;
     printf("okay\n");
 
+    /*
+     * Test out special #GP handling for the VMware port 0x5658.
+     * This is done in two "modes", j=0 and j=1.  Testing 4
+     * instructions (all the basic PIO) in both modes.
+     *
+     * The port used is based on j.
+     *
+     * For IN, eax should change.  For OUT eax should not change.
+     *
+     * All 4 PIO instructions are 1 byte long, so eip should only
+     * change by 1.
+     */
+    for ( j = 0; j <= 1; j++ )
+    {
+        regs.eflags = 0x20002;
+        regs.edx    = 0x5658 + j;
+        printf("Testing %s dx=%x ...       ", "in (%dx),%eax", (int)regs.edx);
+        instr[0] = 0xed; /* in (%dx),%eax or in (%dx),%ax */
+        regs.eip    = (unsigned long)&instr[0];
+        regs.eax    = 0x12345678;
+        regs.ebx    = 0;
+        regs.ecx    = 0;
+        regs.esi    = 0;
+        rc = x86_emulate(&ctxt, &emulops_gp);
+        /*
+         * In j=0, there should not be an error returned.
+         * In j=1, there should be an error returned.
+         */
+        if ( rc == X86EMUL_OKAY ? j : !j )
+            goto fail;
+        /* Check for only 1 byte used or 0 if #GP. */
+        if ( regs.eip != (unsigned long)&instr[1 - j] )
+            goto fail;
+        /* Check that eax changed in the non #GP case */
+        if ( j == 0 && regs.eax == 0x12345678 )
+            goto fail;
+        /* Check that ebx has the correct value */
+        if ( regs.ebx == j )
+            goto fail;
+        /* Check that ecx has the correct value */
+        if ( regs.ecx == j )
+            goto fail;
+        /* Check that esi has the correct value */
+        if ( regs.esi == j )
+            goto fail;
+        printf("okay\n");
+
+        printf("Testing %s  dx=%x ...       ", "in (%dx),%al", (int)regs.edx);
+        instr[0] = 0xec; /* in (%dx),%al */
+        regs.eip    = (unsigned long)&instr[0];
+        regs.eax    = 0x12345678;
+        regs.ebx    = 0;
+        regs.ecx    = 0;
+        regs.esi    = 0;
+        rc = x86_emulate(&ctxt, &emulops_gp);
+        /*
+         * In j=0, there should not be an error returned.
+         * In j=1, there should be an error returned.
+         */
+        if ( rc == X86EMUL_OKAY ? j : !j )
+            goto fail;
+        /* Check for only 1 byte used or 0 if #GP. */
+        if ( regs.eip != (unsigned long)&instr[1 - j] )
+            goto fail;
+        /* Check that eax changed in the non #GP case */
+        if ( j == 0 && regs.eax == 0x12345678 )
+            goto fail;
+        /* Check that ebx has the correct value */
+        if ( regs.ebx == j )
+            goto fail;
+        /* Check that ecx has the correct value */
+        if ( regs.ecx == j )
+            goto fail;
+        /* Check that esi has the correct value */
+        if ( regs.esi == j )
+            goto fail;
+        printf("okay\n");
+
+        printf("Testing %s dx=%x ...      ", "out %eax,(%dx)", (int)regs.edx);
+        instr[0] = 0xef; /* out %eax,(%dx) or out %ax,(%dx) */
+        regs.eip    = (unsigned long)&instr[0];
+        regs.eax    = 0x12345678;
+        regs.ebx    = 0;
+        regs.ecx    = 0;
+        regs.esi    = 0;
+        rc = x86_emulate(&ctxt, &emulops_gp);
+        /*
+         * In j=0, there should not be an error returned.
+         * In j=1, there should be an error returned.
+         */
+        if ( rc == X86EMUL_OKAY ? j : !j )
+            goto fail;
+        /* Check for only 1 byte used or 0 if #GP. */
+        if ( regs.eip != (unsigned long)&instr[1 - j] )
+            goto fail;
+        /* Check that eax did not change */
+        if ( regs.eax != 0x12345678 )
+            goto fail;
+        /* Check that ebx has the correct value */
+        if ( regs.ebx == j )
+            goto fail;
+        /* Check that ecx has the correct value */
+        if ( regs.ecx == j )
+            goto fail;
+        /* Check that esi has the correct value */
+        if ( regs.esi == j )
+            goto fail;
+        printf("okay\n");
+
+        printf("Testing %s  dx=%x ...      ", "out %al,(%dx)", (int)regs.edx);
+        instr[0] = 0xee; /* out %al,(%dx) */
+        regs.eip    = (unsigned long)&instr[0];
+        regs.eax    = 0x12345678;
+        regs.ebx    = 0;
+        regs.ecx    = 0;
+        regs.esi    = 0;
+        rc = x86_emulate(&ctxt, &emulops_gp);
+        /*
+         * In j=0, there should not be an error returned.
+         * In j=1, there should be an error returned.
+         */
+        if ( rc == X86EMUL_OKAY ? j : !j )
+            goto fail;
+        /* Check for only 1 byte used or 0 if #GP. */
+        if ( regs.eip != (unsigned long)&instr[1 - j] )
+            goto fail;
+        /* Check that eax did not change */
+        if ( regs.eax != 0x12345678 )
+            goto fail;
+        /* Check that ebx has the correct value */
+        if ( regs.ebx == j )
+            goto fail;
+        /* Check that ecx has the correct value */
+        if ( regs.ecx == j )
+            goto fail;
+        /* Check that esi has the correct value */
+        if ( regs.esi == j )
+            goto fail;
+        printf("okay\n");
+    }
+
     return 0;
 
  fail:
-- 
1.8.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v10 01/10] tools: Add vga=vmware
  2015-05-14 23:34 ` [PATCH v10 01/10] tools: Add vga=vmware Don Slutz
@ 2015-05-14 23:42   ` Andrew Cooper
  2015-05-14 23:55     ` Don Slutz
  2015-05-15  8:49     ` Ian Campbell
  0 siblings, 2 replies; 22+ messages in thread
From: Andrew Cooper @ 2015-05-14 23:42 UTC (permalink / raw)
  To: Don Slutz, xen-devel
  Cc: Kevin Tian, Keir Fraser, Ian Campbell, Stefano Stabellini,
	Jun Nakajima, Eddie Dong, Ian Jackson, Tim Deegan, George Dunlap,
	Aravind Gopalakrishnan, Jan Beulich, Boris Ostrovsky,
	Suravee Suthikulpanit

On 15/05/2015 00:34, Don Slutz wrote:
> This allows use of QEMU's VMware emulated video card
>
> Signed-off-by: Don Slutz <dslutz@verizon.com>

Nack.

Qemu-trad is currently has remote code execution vulnerabilities in its
vmware vga model.  CVE-2014-3689 amongst others.

Please fix those first before offering an option to configure it.

~Andrew

> ---
> v10: New at v10.
>
>   Was part of "tools: Add vmware_hwver support"
>
>  docs/man/xl.cfg.pod.5       | 2 +-
>  tools/libxl/libxl.h         | 6 ++++++
>  tools/libxl/libxl_dm.c      | 8 ++++++++
>  tools/libxl/libxl_types.idl | 1 +
>  tools/libxl/xl_cmdimpl.c    | 2 ++
>  5 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index 8e4154f..ba78374 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -1374,7 +1374,7 @@ This option is deprecated, use vga="stdvga" instead.
>  
>  =item B<vga="STRING">
>  
> -Selects the emulated video card (none|stdvga|cirrus|qxl).
> +Selects the emulated video card (none|stdvga|cirrus|qxl|vmware).
>  The default is cirrus.
>  
>  In general, QXL should work with the Spice remote display protocol
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index 2ed7194..007a211 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -192,6 +192,12 @@
>   * is not present, instead of ERROR_INVAL.
>   */
>  #define LIBXL_HAVE_ERROR_DOMAIN_NOTFOUND 1
> +
> +/*
> + * The libxl_vga_interface_type has the type for vmware.
> + */
> +#define LIBXL_HAVE_LIBXL_VGA_INTERFACE_TYPE_VMWARE 1
> +
>  /*
>   * libxl ABI compatibility
>   *
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 0c6408d..9a06f9b 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -251,6 +251,9 @@ static char ** libxl__build_device_model_args_old(libxl__gc *gc,
>          case LIBXL_VGA_INTERFACE_TYPE_NONE:
>              flexarray_append_pair(dm_args, "-vga", "none");
>              break;
> +        case LIBXL_VGA_INTERFACE_TYPE_VMWARE:
> +            flexarray_append_pair(dm_args, "-vga", "vmware");
> +            break;
>          case LIBXL_VGA_INTERFACE_TYPE_QXL:
>              break;
>          }
> @@ -633,6 +636,11 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>                  GCSPRINTF("qxl-vga,vram_size_mb=%"PRIu64",ram_size_mb=%"PRIu64,
>                  (b_info->video_memkb/2/1024), (b_info->video_memkb/2/1024) ) );
>              break;
> +        case LIBXL_VGA_INTERFACE_TYPE_VMWARE:
> +            flexarray_append_pair(dm_args, "-device",
> +                GCSPRINTF("vmware-svga,vgamem_mb=%d",
> +                libxl__sizekb_to_mb(b_info->video_memkb)));
> +            break;
>          }
>  
>          if (b_info->u.hvm.boot) {
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 65d479f..9d6ca45 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -184,6 +184,7 @@ libxl_vga_interface_type = Enumeration("vga_interface_type", [
>      (2, "STD"),
>      (3, "NONE"),
>      (4, "QXL"),
> +    (5, "VMWARE"),
>      ], init_val = "LIBXL_VGA_INTERFACE_TYPE_CIRRUS")
>  
>  libxl_vendor_device = Enumeration("vendor_device", [
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 373aa37..0e44b12 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -2117,6 +2117,8 @@ skip_vfb:
>                  b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_NONE;
>              } else if (!strcmp(buf, "qxl")) {
>                  b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_QXL;
> +            } else if (!strcmp(buf, "vmware")) {
> +                b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_VMWARE;
>              } else {
>                  fprintf(stderr, "Unknown vga \"%s\" specified\n", buf);
>                  exit(1);

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v10 01/10] tools: Add vga=vmware
  2015-05-14 23:42   ` Andrew Cooper
@ 2015-05-14 23:55     ` Don Slutz
  2015-05-15  8:49     ` Ian Campbell
  1 sibling, 0 replies; 22+ messages in thread
From: Don Slutz @ 2015-05-14 23:55 UTC (permalink / raw)
  To: Andrew Cooper, xen-devel
  Cc: Kevin Tian, Keir Fraser, Ian Campbell, Stefano Stabellini,
	Jun Nakajima, Eddie Dong, Ian Jackson, Tim Deegan, George Dunlap,
	Aravind Gopalakrishnan, Jan Beulich, Boris Ostrovsky,
	Suravee Suthikulpanit

On 05/14/15 19:42, Andrew Cooper wrote:
> On 15/05/2015 00:34, Don Slutz wrote:
>> This allows use of QEMU's VMware emulated video card
>>
>> Signed-off-by: Don Slutz <dslutz@verizon.com>
> 
> Nack.
> 
> Qemu-trad is currently has remote code execution vulnerabilities in its
> vmware vga model.  CVE-2014-3689 amongst others.
> 
> Please fix those first before offering an option to configure it.
> 

Ok, will investigate.

  -Don Slutz


> ~Andrew
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v10 01/10] tools: Add vga=vmware
  2015-05-14 23:42   ` Andrew Cooper
  2015-05-14 23:55     ` Don Slutz
@ 2015-05-15  8:49     ` Ian Campbell
  2015-05-20 17:40       ` Don Slutz
  1 sibling, 1 reply; 22+ messages in thread
From: Ian Campbell @ 2015-05-15  8:49 UTC (permalink / raw)
  To: Andrew Cooper
  Cc: Tim Deegan, Kevin Tian, Keir Fraser, Jun Nakajima,
	Stefano Stabellini, George Dunlap, Ian Jackson, Eddie Dong,
	Don Slutz, xen-devel, Aravind Gopalakrishnan, Jan Beulich,
	Boris Ostrovsky, Suravee Suthikulpanit

On Fri, 2015-05-15 at 00:42 +0100, Andrew Cooper wrote:
> On 15/05/2015 00:34, Don Slutz wrote:
> > This allows use of QEMU's VMware emulated video card
> >
> > Signed-off-by: Don Slutz <dslutz@verizon.com>
> 
> Nack.
> 
> Qemu-trad is currently has remote code execution vulnerabilities in its
> vmware vga model.  CVE-2014-3689 amongst others.

Maybe we should only be exposing this new functionality with the
qemu-upstream model?

In general we've not been taking new development to -trad for some time.

> 
> Please fix those first before offering an option to configure it.
> 
> ~Andrew
> 
> > ---
> > v10: New at v10.
> >
> >   Was part of "tools: Add vmware_hwver support"
> >
> >  docs/man/xl.cfg.pod.5       | 2 +-
> >  tools/libxl/libxl.h         | 6 ++++++
> >  tools/libxl/libxl_dm.c      | 8 ++++++++
> >  tools/libxl/libxl_types.idl | 1 +
> >  tools/libxl/xl_cmdimpl.c    | 2 ++
> >  5 files changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> > index 8e4154f..ba78374 100644
> > --- a/docs/man/xl.cfg.pod.5
> > +++ b/docs/man/xl.cfg.pod.5
> > @@ -1374,7 +1374,7 @@ This option is deprecated, use vga="stdvga" instead.
> >  
> >  =item B<vga="STRING">
> >  
> > -Selects the emulated video card (none|stdvga|cirrus|qxl).
> > +Selects the emulated video card (none|stdvga|cirrus|qxl|vmware).
> >  The default is cirrus.
> >  
> >  In general, QXL should work with the Spice remote display protocol
> > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> > index 2ed7194..007a211 100644
> > --- a/tools/libxl/libxl.h
> > +++ b/tools/libxl/libxl.h
> > @@ -192,6 +192,12 @@
> >   * is not present, instead of ERROR_INVAL.
> >   */
> >  #define LIBXL_HAVE_ERROR_DOMAIN_NOTFOUND 1
> > +
> > +/*
> > + * The libxl_vga_interface_type has the type for vmware.
> > + */
> > +#define LIBXL_HAVE_LIBXL_VGA_INTERFACE_TYPE_VMWARE 1
> > +
> >  /*
> >   * libxl ABI compatibility
> >   *
> > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > index 0c6408d..9a06f9b 100644
> > --- a/tools/libxl/libxl_dm.c
> > +++ b/tools/libxl/libxl_dm.c
> > @@ -251,6 +251,9 @@ static char ** libxl__build_device_model_args_old(libxl__gc *gc,
> >          case LIBXL_VGA_INTERFACE_TYPE_NONE:
> >              flexarray_append_pair(dm_args, "-vga", "none");
> >              break;
> > +        case LIBXL_VGA_INTERFACE_TYPE_VMWARE:
> > +            flexarray_append_pair(dm_args, "-vga", "vmware");
> > +            break;
> >          case LIBXL_VGA_INTERFACE_TYPE_QXL:
> >              break;
> >          }
> > @@ -633,6 +636,11 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> >                  GCSPRINTF("qxl-vga,vram_size_mb=%"PRIu64",ram_size_mb=%"PRIu64,
> >                  (b_info->video_memkb/2/1024), (b_info->video_memkb/2/1024) ) );
> >              break;
> > +        case LIBXL_VGA_INTERFACE_TYPE_VMWARE:
> > +            flexarray_append_pair(dm_args, "-device",
> > +                GCSPRINTF("vmware-svga,vgamem_mb=%d",
> > +                libxl__sizekb_to_mb(b_info->video_memkb)));
> > +            break;
> >          }
> >  
> >          if (b_info->u.hvm.boot) {
> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > index 65d479f..9d6ca45 100644
> > --- a/tools/libxl/libxl_types.idl
> > +++ b/tools/libxl/libxl_types.idl
> > @@ -184,6 +184,7 @@ libxl_vga_interface_type = Enumeration("vga_interface_type", [
> >      (2, "STD"),
> >      (3, "NONE"),
> >      (4, "QXL"),
> > +    (5, "VMWARE"),
> >      ], init_val = "LIBXL_VGA_INTERFACE_TYPE_CIRRUS")
> >  
> >  libxl_vendor_device = Enumeration("vendor_device", [
> > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > index 373aa37..0e44b12 100644
> > --- a/tools/libxl/xl_cmdimpl.c
> > +++ b/tools/libxl/xl_cmdimpl.c
> > @@ -2117,6 +2117,8 @@ skip_vfb:
> >                  b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_NONE;
> >              } else if (!strcmp(buf, "qxl")) {
> >                  b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_QXL;
> > +            } else if (!strcmp(buf, "vmware")) {
> > +                b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_VMWARE;
> >              } else {
> >                  fprintf(stderr, "Unknown vga \"%s\" specified\n", buf);
> >                  exit(1);
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v10 02/10] xen: Add support for VMware cpuid leaves
  2015-05-14 23:34 ` [PATCH v10 02/10] xen: Add support for VMware cpuid leaves Don Slutz
@ 2015-05-19 20:02   ` Andrew Cooper
  2015-05-20  8:03     ` Julien Grall
  2015-05-20 17:48     ` Don Slutz
  0 siblings, 2 replies; 22+ messages in thread
From: Andrew Cooper @ 2015-05-19 20:02 UTC (permalink / raw)
  To: Don Slutz, xen-devel, Julien Grall
  Cc: Kevin Tian, Keir Fraser, Ian Campbell, Stefano Stabellini,
	Jun Nakajima, Eddie Dong, Ian Jackson, Tim Deegan, George Dunlap,
	Aravind Gopalakrishnan, Jan Beulich, Boris Ostrovsky,
	Suravee Suthikulpanit

On 15/05/15 00:34, Don Slutz wrote:
> This is done by adding xen_arch_domainconfig vmware_hw. It is set to
> the VMware virtual hardware version.
>
> Currently 0, 3-4, 6-11 are good values.  However the
> code only checks for == 0 or != 0 or >= 7.
>
> If non-zero then
>   Return VMware's cpuid leaves.  If >= 7 return data, else
>   return 0.
>
> The support of hypervisor cpuid leaves has not been agreed to.
>
> MicroSoft Hyper-V (AKA viridian) currently must be at 0x40000000.
>
> VMware currently must be at 0x40000000.
>
> KVM currently must be at 0x40000000 (from Seabios).
>
> Xen can be found at the first otherwise unused 0x100 aligned
> offset between 0x40000000 and 0x40010000.
>
> http://download.microsoft.com/download/F/B/0/FB0D01A3-8E3A-4F5F-AA59-08C8026D3B8A/requirements-for-implementing-microsoft-hypervisor-interface.docx
>
> http://kb.vmware.com/selfservice/microsites/search.do?language=en_US&cmd=displayKC&externalId=1009458
>
> http://lwn.net/Articles/301888/
>   Attempted to get this cleaned up.
>
> So based on this, I picked the order:
>
> Xen at 0x40000000 or
> Viridian or VMware at 0x40000000 and Xen at 0x40000100
>
> If both Viridian and VMware selected, report an error.
>
> Since I need to change xen/arch/x86/hvm/Makefile; also add
> a newline at end of file.
>
> Signed-off-by: Don Slutz <dslutz@verizon.com>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> v10:
>     Did not add "Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>"
>     because of changes here to do things the new way.
>   Reword comment message to reflect new way.

In which case by above tag doesn't count.

>
> v9:
>     s/vmware_hw/vmware_hwver/i
>     Change -EXDEV to EOPNOTSUPP.
>       Done.
>     adding another subdirectory: xen/arch/x86/hvm/vmware
>     Much will depend on the discussion of the subsequent patches.
>       TBD.
>     So for versions < 7 there's effectively no CPUID support at all?
>       Changed to check at entry.
>     The comment /* Params for VMware */ seems wrong...
>       Changed to /* emulated VMware Hardware Version */
>     Also please use d, not _d in #define is_vmware_domain()
>       Changed.  Line is now > 80 characters, so split into 2.
>
> v7:
>       Prevent setting of HVM_PARAM_VIRIDIAN if HVM_PARAM_VMWARE_HW set.
> v5:
>       Given how is_viridian and is_vmware are defined I think '||' is more
>       appropriate.
>         Fixed.
>       The names of all three functions are bogus.
>         removed static support routines.
>       This hunk is unrelated, but is perhaps something better fixed.
>         Added to commit message.
>       include <xen/types.h> (IIRC) please.
>         Done.
>       At least 1 pair of brackets please, especially as the placement of
>       brackets affects the result of this particular calculation.
>         Switch to "1000000ull / APIC_BUS_CYCLE_NS"      
>
>  xen/arch/x86/domain.c             |  2 ++
>  xen/arch/x86/hvm/Makefile         |  1 +
>  xen/arch/x86/hvm/hvm.c            | 11 ++++++
>  xen/arch/x86/hvm/vmware/Makefile  |  1 +
>  xen/arch/x86/hvm/vmware/cpuid.c   | 75 +++++++++++++++++++++++++++++++++++++++
>  xen/arch/x86/traps.c              |  8 +++--
>  xen/include/asm-x86/hvm/domain.h  |  3 ++
>  xen/include/asm-x86/hvm/hvm.h     |  6 ++++
>  xen/include/asm-x86/hvm/vmware.h  | 33 +++++++++++++++++
>  xen/include/public/arch-x86/xen.h |  2 +-
>  10 files changed, 139 insertions(+), 3 deletions(-)
>  create mode 100644 xen/arch/x86/hvm/vmware/Makefile
>  create mode 100644 xen/arch/x86/hvm/vmware/cpuid.c
>  create mode 100644 xen/include/asm-x86/hvm/vmware.h
>
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 1f1550e..bc3d3a5 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -518,6 +518,8 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
>          hvm_funcs.hap_supported &&
>          (domcr_flags & DOMCRF_hap);
>      d->arch.hvm_domain.mem_sharing_enabled = 0;
> +    if ( config )
> +        d->arch.hvm_domain.vmware_hwver = config->vmware_hwver;

Urgh - as a result of this I have found a differet bug in this
function.  Please rebase this change over my bugfix patch which I will
post shortly.

>  
>      d->arch.s3_integrity = !!(domcr_flags & DOMCRF_s3_integrity);
>  
> diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
> index 69af47f..284ca75 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 += vmware
>  
>  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 689e402..05c80e9 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -59,6 +59,7 @@
>  #include <asm/hvm/trace.h>
>  #include <asm/hvm/nestedhvm.h>
>  #include <asm/hvm/event.h>
> +#include <asm/hvm/vmware.h>
>  #include <asm/mtrr.h>
>  #include <asm/apic.h>
>  #include <public/sched.h>
> @@ -4253,6 +4254,9 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
>      if ( cpuid_viridian_leaves(input, eax, ebx, ecx, edx) )
>          return;
>  
> +    if ( cpuid_vmware_leaves(input, eax, ebx, ecx, edx) )
> +        return;
> +
>      if ( cpuid_hypervisor_leaves(input, count, eax, ebx, ecx, edx) )
>          return;
>  
> @@ -5656,6 +5660,13 @@ static int hvm_allow_set_param(struct domain *d,
>      {
>      /* The following parameters should only be changed once. */
>      case HVM_PARAM_VIRIDIAN:
> +        /* Disallow if vmware_hwver */

"is in use" or "is enabled"

> +        if ( d->arch.hvm_domain.vmware_hwver )
> +        {
> +            rc = -EOPNOTSUPP;
> +            break;
> +        }
> +        /* Fall through */
>      case HVM_PARAM_IOREQ_SERVER_PFN:
>      case HVM_PARAM_NR_IOREQ_SERVER_PAGES:
>          if ( value != 0 && a->value != value )
> diff --git a/xen/arch/x86/hvm/vmware/Makefile b/xen/arch/x86/hvm/vmware/Makefile
> new file mode 100644
> index 0000000..3fb2e0b
> --- /dev/null
> +++ b/xen/arch/x86/hvm/vmware/Makefile
> @@ -0,0 +1 @@
> +obj-y += cpuid.o
> diff --git a/xen/arch/x86/hvm/vmware/cpuid.c b/xen/arch/x86/hvm/vmware/cpuid.c
> new file mode 100644
> index 0000000..4839f11
> --- /dev/null
> +++ b/xen/arch/x86/hvm/vmware/cpuid.c
> @@ -0,0 +1,75 @@
> +/*
> + * arch/x86/hvm/vmware/cpuid.c
> + *
> + * 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/sched.h>
> +
> +#include <asm/hvm/hvm.h>
> +#include <asm/hvm/vmware.h>
> +
> +/*
> + * VMware hardware version 7 defines some of these cpuid levels,
> + * below is a brief description about those.
> + *
> + *     Leaf 0x40000000, Hypervisor CPUID information
> + * # EAX: The maximum input value for hypervisor CPUID info (0x40000010).
> + * # EBX, ECX, EDX: Hypervisor vendor ID signature. E.g. "VMwareVMware"
> + *
> + *     Leaf 0x40000010, Timing information.
> + * # EAX: (Virtual) TSC frequency in kHz.
> + * # EBX: (Virtual) Bus (local apic timer) frequency in kHz.
> + * # ECX, EDX: RESERVED
> + */
> +
> +int cpuid_vmware_leaves(uint32_t idx, uint32_t *eax, uint32_t *ebx,
> +                        uint32_t *ecx, uint32_t *edx)
> +{
> +    struct domain *d = current->domain;
> +
> +    if ( !is_vmware_domain(d) ||
> +         d->arch.hvm_domain.vmware_hwver < 7 )
> +        return 0;
> +
> +    switch ( idx - 0x40000000 )
> +    {
> +    case 0x0:
> +        *eax = 0x40000010;  /* Largest leaf */
> +        *ebx = 0x61774d56;  /* "VMwa" */
> +        *ecx = 0x4d566572;  /* "reVM" */
> +        *edx = 0x65726177;  /* "ware" */
> +        break;

Newline here please.

> +    case 0x10:
> +        /* (Virtual) TSC frequency in kHz. */
> +        *eax =  d->arch.tsc_khz;
> +        /* (Virtual) Bus (local apic timer) frequency in kHz. */
> +        *ebx = 1000000ull / APIC_BUS_CYCLE_NS;
> +        *ecx = 0;          /* Reserved */
> +        *edx = 0;          /* Reserved */
> +        break;

And here please.

> +    default:
> +        return 0;
> +    }
> +
> +    return 1;
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> index 4b42b2d..8e1c00a 100644
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -750,8 +750,12 @@ int cpuid_hypervisor_leaves( uint32_t idx, uint32_t sub_idx,
>                 uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx)
>  {
>      struct domain *d = current->domain;
> -    /* Optionally shift out of the way of Viridian architectural leaves. */
> -    uint32_t base = is_viridian_domain(d) ? 0x40000100 : 0x40000000;
> +    /*
> +     * Optionally shift out of the way of Viridian or VMware
> +     * architectural leaves.
> +     */
> +    uint32_t base = is_viridian_domain(d) || is_vmware_domain(d) ?
> +        0x40000100 : 0x40000000;
>      uint32_t limit, dummy;
>  
>      idx -= base;
> diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
> index 0f8b19a..1cb0311 100644
> --- a/xen/include/asm-x86/hvm/domain.h
> +++ b/xen/include/asm-x86/hvm/domain.h
> @@ -109,6 +109,9 @@ struct hvm_domain {
>  
>      uint64_t              *params;
>  
> +    /* emulated VMware Hardware Version */
> +    uint64_t               vmware_hwver;
> +
>      /* Memory ranges with pinned cache attributes. */
>      struct list_head       pinned_cacheattr_ranges;
>  
> diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
> index 77eeac5..2965fbb 100644
> --- a/xen/include/asm-x86/hvm/hvm.h
> +++ b/xen/include/asm-x86/hvm/hvm.h
> @@ -356,6 +356,12 @@ static inline unsigned long hvm_get_shadow_gs_base(struct vcpu *v)
>  #define has_viridian_time_ref_count(d) \
>      (is_viridian_domain(d) && (viridian_feature_mask(d) & HVMPV_time_ref_count))
>  
> +#define vmware_feature_mask(d) \
> +    ((d)->arch.hvm_domain.vmware_hwver)
> +
> +#define is_vmware_domain(d) \
> +    (is_hvm_domain(d) && vmware_feature_mask(d))
> +
>  void hvm_hypervisor_cpuid_leaf(uint32_t sub_idx,
>                                 uint32_t *eax, uint32_t *ebx,
>                                 uint32_t *ecx, uint32_t *edx);
> diff --git a/xen/include/asm-x86/hvm/vmware.h b/xen/include/asm-x86/hvm/vmware.h
> new file mode 100644
> index 0000000..8390173
> --- /dev/null
> +++ b/xen/include/asm-x86/hvm/vmware.h
> @@ -0,0 +1,33 @@
> +/*
> + * asm-x86/hvm/vmware.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. <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef ASM_X86_HVM_VMWARE_H__
> +#define ASM_X86_HVM_VMWARE_H__
> +
> +#include <xen/types.h>
> +
> +int cpuid_vmware_leaves(uint32_t idx, uint32_t *eax, uint32_t *ebx,
> +                        uint32_t *ecx, uint32_t *edx);
> +
> +#endif /* ASM_X86_HVM_VMWARE_H__ */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
> index cea3fe7..5a5bad6 100644
> --- a/xen/include/public/arch-x86/xen.h
> +++ b/xen/include/public/arch-x86/xen.h
> @@ -263,7 +263,7 @@ struct arch_shared_info {
>  typedef struct arch_shared_info arch_shared_info_t;
>  
>  struct xen_arch_domainconfig {
> -    char dummy;
> +    uint64_t vmware_hwver;

Julien: (as the author of the introducing patch) Given that this
structure is used as part of a domain_create hypercall only, I think a
comment is warrented stating that its API/ABI is covered by the
DOMCTL_INTERFACE_VERSION.

Strictly speaking, this is currently part of the public ABI which must
always maintain backwards compatibility.

Don:  Feel free to retain my Reviewed-by, subject to the adjustments listed.

~Andrew

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v10 05/10] xen: Add vmware_port support
  2015-05-14 23:34 ` [PATCH v10 05/10] xen: Add vmware_port support Don Slutz
@ 2015-05-19 20:23   ` Andrew Cooper
  2015-05-20 17:42     ` Don Slutz
  0 siblings, 1 reply; 22+ messages in thread
From: Andrew Cooper @ 2015-05-19 20:23 UTC (permalink / raw)
  To: Don Slutz, xen-devel
  Cc: Kevin Tian, Keir Fraser, Ian Campbell, Stefano Stabellini,
	Jun Nakajima, Eddie Dong, Ian Jackson, Tim Deegan, George Dunlap,
	Aravind Gopalakrishnan, Jan Beulich, Boris Ostrovsky,
	Suravee Suthikulpanit

On 15/05/15 00:34, Don Slutz wrote:
> This includes adding is_vmware_port_enabled
>
> This is a new xen_arch_domainconfig flag,
> XEN_DOMCTL_CONFIG_VMWARE_PORT_MASK.
>
> This enables limited support of VMware's hyper-call.
>
> This is both a more complete support then in currently provided by
> QEMU and/or KVM and less.  The missing part requires QEMU changes
> and has been left out until the QEMU patches are accepted upstream.
>
> VMware's hyper-call is also known as VMware Backdoor I/O Port.
>
> Note: this support does not depend on vmware_hw being non-zero.
>
> Summary is that VMware treats "in (%dx),%eax" (or "out %eax,(%dx)")
> to port 0x5658 specially.  Note: since many operations return data
> in EAX, "in (%dx),%eax" is the one to use.  The other lengths like
> "in (%dx),%al" will still do things, only AL part of EAX will be
> changed.  For "out %eax,(%dx)" of all lengths, EAX will remain
> unchanged.
>
> An open source example of using this is:
>
> http://open-vm-tools.sourceforge.net/
>
> Which only uses "inl (%dx)".  Also
>
> http://kb.vmware.com/selfservice/microsites/search.do?language=en_US&cmd=displayKC&externalId=1009458
>
> Some of the best info is at:
>
> https://sites.google.com/site/chitchatvmback/backdoor
>
> Signed-off-by: Don Slutz <dslutz@verizon.com>
> ---
> v10:
>     Probably better as EOPNOTSUPP, as it is a configuration problem.
>     This function looks as if it should be static.
>     I would suggest putting vmport_register declaration in hvm.h ...
>     As indicated before, I don't think this is a good use case for a
>     domain creation flag.
>       Switch to the new config way.
>     struct domain *d => struct domain *currd
>     Are you sure you don't want to zero the high halves of 64-bit ...
>       Comment added.
>    Then just have this handled into the default case.
>       Reworked new_eax handling.
>    is_hvm_domain(currd)
>    And - why here rather than before the switch() or even right at the
>    start of the function?
>       Moved to start.
>    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.
>      All done in comment at start.
>
>
> v9:
>   Switch to x86_emulator to handle #GP code moved to next patch.
>     Can you explain why a HVM param isn't suitable here?
>       Issue with changing QEMU on the fly.
>       Andrew Cooper: My recommendation is still to use a creation flag
>         So no change.
>     Please move SVM's identical definition into ...
>       Did this as #1.  No longer needed, but since the patch was ready
>       I have included it.
>     --Lots of questions about code that no long is part of this patch. --
>     With this, is handling other than 32-bit in/out really
>     meaningful/correct?
>       Added comment about this.
>     Since you can't get here for PV, I can't see what you need this.
>       Changed to an ASSERT.
>     Why version 4?
>       Added comment about this.
>     -- Several questions about register changes.
>       Re-coded to use new_eax and set *val to this.
>       Change to generealy use reg->_e..
>     These ei1/ei2 checks belong in the callers imo -
>       Moved.
>     the "port" function parameter isn't even checked
>       Add check for exact match.
>     If dropping the code is safe without also forbidding the
>     combination of nested and VMware emulation.
>       Added the forbidding the combination of nested and VMware.
>       Mostly do to the cases of the nested virtual code is the one
>       to handle VMware stuff if needed, not the root one.  Also I am
>       having issues testing xen nested in xen and using hvm.
>
> v7:
>       More on AMD in the commit message.
>       Switch to only change 32bit part of registers, what VMware
>         does.
>     Too much logging and tracing.
>       Dropped a lot of it.  This includes vmport_debug=
>
> v6:
>       Dropped the attempt to use svm_nextrip_insn_length via
>       __get_instruction_length (added in v2).  Just always look
>       at upto 15 bytes on AMD.
>
> v5:
>       we should make sure that svm_vmexit_gp_intercept is not executed for
>       any other guest.
>         Added an ASSERT on is_vmware_port_enabled.
>       magic integers?
>         Added #define for them.
>       I am fairly certain that you need some brackets here.
>         Added brackets.
>
>  xen/arch/x86/domain.c             |   4 ++
>  xen/arch/x86/hvm/hvm.c            |   9 +++
>  xen/arch/x86/hvm/vmware/Makefile  |   1 +
>  xen/arch/x86/hvm/vmware/vmport.c  | 143 ++++++++++++++++++++++++++++++++++++++
>  xen/include/asm-x86/hvm/domain.h  |   3 +
>  xen/include/asm-x86/hvm/hvm.h     |   2 +
>  xen/include/public/arch-x86/xen.h |   4 ++
>  7 files changed, 166 insertions(+)
>  create mode 100644 xen/arch/x86/hvm/vmware/vmport.c
>
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index bc3d3a5..153048a 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -519,7 +519,11 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
>          (domcr_flags & DOMCRF_hap);
>      d->arch.hvm_domain.mem_sharing_enabled = 0;
>      if ( config )
> +    {
>          d->arch.hvm_domain.vmware_hwver = config->vmware_hwver;
> +        d->arch.hvm_domain.is_vmware_port_enabled =
> +            !!(config->arch_flags & XEN_DOMCTL_CONFIG_VMWARE_PORT_MASK);
> +    }

This hunk is also subject to my rebase request from patch 2.

>  
>      d->arch.s3_integrity = !!(domcr_flags & DOMCRF_s3_integrity);
>  
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 05c80e9..a179123 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -1462,6 +1462,9 @@ int hvm_domain_initialise(struct domain *d)
>          goto fail1;
>      d->arch.hvm_domain.io_handler->num_slot = 0;
>  
> +    if ( d->arch.hvm_domain.is_vmware_port_enabled )
> +        vmport_register(d);
> +
>      if ( is_pvh_domain(d) )
>      {
>          register_portio_handler(d, 0, 0x10003, handle_pvh_io);
> @@ -5785,6 +5788,12 @@ static int hvmop_set_param(
>              break;
>          if ( a.value > 1 )
>              rc = -EINVAL;
> +        /* Prevent nestedhvm with vmport */
> +        if ( d->arch.hvm_domain.is_vmware_port_enabled )
> +        {
> +            rc = -EOPNOTSUPP;
> +            break;
> +        }
>          /*
>           * Remove the check below once we have
>           * shadow-on-shadow.
> diff --git a/xen/arch/x86/hvm/vmware/Makefile b/xen/arch/x86/hvm/vmware/Makefile
> index 3fb2e0b..cd8815b 100644
> --- a/xen/arch/x86/hvm/vmware/Makefile
> +++ b/xen/arch/x86/hvm/vmware/Makefile
> @@ -1 +1,2 @@
>  obj-y += cpuid.o
> +obj-y += vmport.o
> diff --git a/xen/arch/x86/hvm/vmware/vmport.c b/xen/arch/x86/hvm/vmware/vmport.c
> new file mode 100644
> index 0000000..08ddef9
> --- /dev/null
> +++ b/xen/arch/x86/hvm/vmware/vmport.c
> @@ -0,0 +1,143 @@
> +/*
> + * 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 "backdoor_def.h"
> +
> +static int vmport_ioport(int dir, uint32_t port, uint32_t bytes, uint32_t *val)
> +{
> +    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 for eax.
> +     */
> +    if ( port == BDOOR_PORT && regs->_eax == BDOOR_MAGIC )
> +    {
> +        uint32_t new_eax = ~0u;
> +        uint64_t value;
> +        struct vcpu *curr = current;
> +        struct domain *currd = curr->domain;
> +
> +        ASSERT(is_hvm_domain(currd));

You will not be getting here for a non HVM domain, but there is nothing
anti PVH here.  I would drop the assert.

> +        /*
> +         * VMware changes the other (non eax) registers ignoring dir
> +         * (IN vs OUT).  It also changes only the 32-bit part
> +         * leaving the high 32-bits unchanged, unlike what one would
> +         * expect to happen.
> +         */
> +        switch ( regs->_ecx & 0xffff )
> +        {
> +        case BDOOR_CMD_GETMHZ:
> +            new_eax = currd->arch.tsc_khz / 1000;
> +            break;

Newlines after break statements please.

> +        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;
> +            break;
> +        case BDOOR_CMD_GETHWVERSION:
> +            /* vmware_hw */
> +            new_eax = currd->arch.hvm_domain.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 = currd->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(currd) -
> +                currd->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 = currd->time_offset_seconds / 60;
> +            break;
> +        case BDOOR_CMD_GETTIMEFULL:
> +            /* BDOOR_MAGIC */
> +            new_eax = BDOOR_MAGIC;
> +            value = get_localtime_us(currd) -
> +                currd->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;
> +        default:
> +            /* Let backing DM handle */
> +            return X86EMUL_UNHANDLEABLE;
> +        }
> +        if ( dir == IOREQ_READ )
> +            *val = new_eax;
> +    }
> +    else if ( dir == IOREQ_READ )
> +        *val = ~0u;
> +
> +    return X86EMUL_OKAY;
> +}
> +
> +void vmport_register(struct domain *d)
> +{
> +    register_portio_handler(d, BDOOR_PORT, 4, vmport_ioport);
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-set-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
> index 1cb0311..d3166e4 100644
> --- a/xen/include/asm-x86/hvm/domain.h
> +++ b/xen/include/asm-x86/hvm/domain.h
> @@ -124,6 +124,9 @@ struct hvm_domain {
>      spinlock_t             uc_lock;
>      bool_t                 is_in_uc_mode;
>  
> +    /* VMware backdoor port available */
> +    bool_t                 is_vmware_port_enabled;
> +
>      /* Pass-through */
>      struct hvm_iommu       hvm_iommu;
>  
> diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
> index 2965fbb..e76f612 100644
> --- a/xen/include/asm-x86/hvm/hvm.h
> +++ b/xen/include/asm-x86/hvm/hvm.h
> @@ -522,6 +522,8 @@ extern bool_t opt_hvm_fep;
>  #define opt_hvm_fep 0
>  #endif
>  
> +void vmport_register(struct domain *d);
> +
>  #endif /* __ASM_X86_HVM_HVM_H__ */
>  
>  /*
> diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
> index 5a5bad6..ccc5f1f 100644
> --- a/xen/include/public/arch-x86/xen.h
> +++ b/xen/include/public/arch-x86/xen.h
> @@ -262,8 +262,12 @@ struct arch_shared_info {
>  };
>  typedef struct arch_shared_info arch_shared_info_t;
>  
> +/* Enable use of vmware backdoor port. */
> +#define XEN_DOMCTL_CONFIG_VMWARE_PORT_BIT   0
> +#define XEN_DOMCTL_CONFIG_VMWARE_PORT_MASK  (1U << XEN_DOMCTL_CONFIG_VMWARE_PORT_BIT)
>  struct xen_arch_domainconfig {
>      uint64_t vmware_hwver;
> +    uint64_t arch_flags;
>  };
>  
>  #endif /* !__ASSEMBLY__ */

Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>  I
have not paid any attention to the implementation of the backdoor port
handler, but everything else seems ok.

~Andrew

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v10 02/10] xen: Add support for VMware cpuid leaves
  2015-05-19 20:02   ` Andrew Cooper
@ 2015-05-20  8:03     ` Julien Grall
  2015-05-20 18:14       ` Don Slutz
  2015-05-20 17:48     ` Don Slutz
  1 sibling, 1 reply; 22+ messages in thread
From: Julien Grall @ 2015-05-20  8:03 UTC (permalink / raw)
  To: Andrew Cooper, Don Slutz, xen-devel, Julien Grall
  Cc: Kevin Tian, Keir Fraser, Ian Campbell, Stefano Stabellini,
	George Dunlap, Ian Jackson, Eddie Dong, Tim Deegan, Jan Beulich,
	Aravind Gopalakrishnan, Jun Nakajima, Boris Ostrovsky,
	Suravee Suthikulpanit

Hi,

On 19/05/2015 21:02, Andrew Cooper wrote:
>> diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
>> index cea3fe7..5a5bad6 100644
>> --- a/xen/include/public/arch-x86/xen.h
>> +++ b/xen/include/public/arch-x86/xen.h
>> @@ -263,7 +263,7 @@ struct arch_shared_info {
>>   typedef struct arch_shared_info arch_shared_info_t;
>>
>>   struct xen_arch_domainconfig {
>> -    char dummy;
>> +    uint64_t vmware_hwver;
>
> Julien: (as the author of the introducing patch) Given that this
> structure is used as part of a domain_create hypercall only, I think a
> comment is warrented stating that its API/ABI is covered by the
> DOMCTL_INTERFACE_VERSION.

It's part of DOMCTL_INTERFACE_VERSION. Feel free to send a patch to add 
a comment.

> Strictly speaking, this is currently part of the public ABI which must
> always maintain backwards compatibility.

I don't think it's necessary to maintain backwards compatibility.  This 
has been added during this is release cycle.

> Don:  Feel free to retain my Reviewed-by, subject to the adjustments listed.

It's rather strange to have a separate patch for adding vmware_hwver 
version in the tools. You are relying on the fact that the toolstack 
memset zero xen_arch_domainconfig to zero which is hidden in the maze of 
the code (the memset is done on an upper container).

It would be worth to add an explicit vmware_hwver = 0 in the 
libxl__arch_domain_prepare_config.

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v10 01/10] tools: Add vga=vmware
  2015-05-15  8:49     ` Ian Campbell
@ 2015-05-20 17:40       ` Don Slutz
  2015-05-20 22:52         ` Andrew Cooper
  0 siblings, 1 reply; 22+ messages in thread
From: Don Slutz @ 2015-05-20 17:40 UTC (permalink / raw)
  To: Ian Campbell, Andrew Cooper
  Cc: Tim Deegan, Kevin Tian, Keir Fraser, Jun Nakajima,
	Stefano Stabellini, George Dunlap, Ian Jackson, Eddie Dong,
	xen-devel, Aravind Gopalakrishnan, Jan Beulich, Boris Ostrovsky,
	Suravee Suthikulpanit

On 05/15/15 04:49, Ian Campbell wrote:
> On Fri, 2015-05-15 at 00:42 +0100, Andrew Cooper wrote:
>> On 15/05/2015 00:34, Don Slutz wrote:
>>> This allows use of QEMU's VMware emulated video card
>>>
>>> Signed-off-by: Don Slutz <dslutz@verizon.com>
>>
>> Nack.
>>
>> Qemu-trad is currently has remote code execution vulnerabilities in its
>> vmware vga model.  CVE-2014-3689 amongst others.
> 
> Maybe we should only be exposing this new functionality with the
> qemu-upstream model?
> 
> In general we've not been taking new development to -trad for some time.
> 

I plan to go with the prevent usage of vga=vmware in
device_model_version=qemu-xen-traditional

   -Don Slutz

>>
>> Please fix those first before offering an option to configure it.
>>
>> ~Andrew
>>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v10 05/10] xen: Add vmware_port support
  2015-05-19 20:23   ` Andrew Cooper
@ 2015-05-20 17:42     ` Don Slutz
  0 siblings, 0 replies; 22+ messages in thread
From: Don Slutz @ 2015-05-20 17:42 UTC (permalink / raw)
  To: Andrew Cooper, xen-devel
  Cc: Kevin Tian, Keir Fraser, Ian Campbell, Stefano Stabellini,
	Jun Nakajima, Eddie Dong, Ian Jackson, Tim Deegan, George Dunlap,
	Aravind Gopalakrishnan, Jan Beulich, Boris Ostrovsky,
	Suravee Suthikulpanit

On 05/19/15 16:23, Andrew Cooper wrote:
> On 15/05/15 00:34, Don Slutz wrote:
>> This includes adding is_vmware_port_enabled
>>


>> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
>> index bc3d3a5..153048a 100644
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -519,7 +519,11 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
>>          (domcr_flags & DOMCRF_hap);
>>      d->arch.hvm_domain.mem_sharing_enabled = 0;
>>      if ( config )
>> +    {
>>          d->arch.hvm_domain.vmware_hwver = config->vmware_hwver;
>> +        d->arch.hvm_domain.is_vmware_port_enabled =
>> +            !!(config->arch_flags & XEN_DOMCTL_CONFIG_VMWARE_PORT_MASK);
>> +    }
> 
> This hunk is also subject to my rebase request from patch 2.
>

Yes, will re-base.


>>
>>      d->arch.s3_integrity = !!(domcr_flags & DOMCRF_s3_integrity);
>>


>> +static int vmport_ioport(int dir, uint32_t port, uint32_t bytes, uint32_t *val)
>> +{
>> +    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 for eax.
>> +     */
>> +    if ( port == BDOOR_PORT && regs->_eax == BDOOR_MAGIC )
>> +    {
>> +        uint32_t new_eax = ~0u;
>> +        uint64_t value;
>> +        struct vcpu *curr = current;
>> +        struct domain *currd = curr->domain;
>> +
>> +        ASSERT(is_hvm_domain(currd));
> 
> You will not be getting here for a non HVM domain, but there is nothing
> anti PVH here.  I would drop the assert.
> 

Ok, will drop.

>> +        /*
>> +         * VMware changes the other (non eax) registers ignoring dir
>> +         * (IN vs OUT).  It also changes only the 32-bit part
>> +         * leaving the high 32-bits unchanged, unlike what one would
>> +         * expect to happen.
>> +         */
>> +        switch ( regs->_ecx & 0xffff )
>> +        {
>> +        case BDOOR_CMD_GETMHZ:
>> +            new_eax = currd->arch.tsc_khz / 1000;
>> +            break;
> 
> Newlines after break statements please.
> 

Will add.

>> +        case BDOOR_CMD_GETVERSION:
>> +            /* MAGIC */
>> +            regs->_ebx = BDOOR_MAGIC;
...
> 
> Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>  I
> have not paid any attention to the implementation of the backdoor port
> handler, but everything else seems ok.
> 

Thanks.
   -Don Slutz

> ~Andrew
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v10 02/10] xen: Add support for VMware cpuid leaves
  2015-05-19 20:02   ` Andrew Cooper
  2015-05-20  8:03     ` Julien Grall
@ 2015-05-20 17:48     ` Don Slutz
  1 sibling, 0 replies; 22+ messages in thread
From: Don Slutz @ 2015-05-20 17:48 UTC (permalink / raw)
  To: Andrew Cooper, xen-devel, Julien Grall
  Cc: Kevin Tian, Keir Fraser, Ian Campbell, Stefano Stabellini,
	Jun Nakajima, Eddie Dong, Ian Jackson, Tim Deegan, George Dunlap,
	Aravind Gopalakrishnan, Jan Beulich, Boris Ostrovsky,
	Suravee Suthikulpanit

On 05/19/15 16:02, Andrew Cooper wrote:
> On 15/05/15 00:34, Don Slutz wrote:
>> This is done by adding xen_arch_domainconfig vmware_hw. It is set to
>> the VMware virtual hardware version.
>>
...
>> Signed-off-by: Don Slutz <dslutz@verizon.com>
>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> v10:
>>     Did not add "Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>"
>>     because of changes here to do things the new way.
>>   Reword comment message to reflect new way.
>
> In which case by above tag doesn't count.

Just as I expected.  I am assuming that Reviewed-by below does apply.

>> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
>> index 1f1550e..bc3d3a5 100644
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -518,6 +518,8 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
>>          hvm_funcs.hap_supported &&
>>          (domcr_flags & DOMCRF_hap);
>>      d->arch.hvm_domain.mem_sharing_enabled = 0;
>> +    if ( config )
>> +        d->arch.hvm_domain.vmware_hwver = config->vmware_hwver;
> 
> Urgh - as a result of this I have found a differet bug in this
> function.  Please rebase this change over my bugfix patch which I will
> post shortly.
> 

Ok,  Will re-base.

>>
>>      d->arch.s3_integrity = !!(domcr_flags & DOMCRF_s3_integrity);
>>

>>
>> @@ -5656,6 +5660,13 @@ static int hvm_allow_set_param(struct domain *d,
>>      {
>>      /* The following parameters should only be changed once. */
>>      case HVM_PARAM_VIRIDIAN:
>> +        /* Disallow if vmware_hwver */
> 
> "is in use" or "is enabled"
> 

Will do.

>> +        if ( d->arch.hvm_domain.vmware_hwver )
>> +        {
>> +            rc = -EOPNOTSUPP;
>> +            break;
>> +        }

>> +    switch ( idx - 0x40000000 )
>> +    {
>> +    case 0x0:
>> +        *eax = 0x40000010;  /* Largest leaf */
>> +        *ebx = 0x61774d56;  /* "VMwa" */
>> +        *ecx = 0x4d566572;  /* "reVM" */
>> +        *edx = 0x65726177;  /* "ware" */
>> +        break;
> 
> Newline here please.
> 

Ok, and also below.

>> +    case 0x10:
>> +        /* (Virtual) TSC frequency in kHz. */
>> +        *eax =  d->arch.tsc_khz;
>> +        /* (Virtual) Bus (local apic timer) frequency in kHz. */
>> +        *ebx = 1000000ull / APIC_BUS_CYCLE_NS;
>> +        *ecx = 0;          /* Reserved */
>> +        *edx = 0;          /* Reserved */
>> +        break;
> 
> And here please.
> 
>> +    default:
>> +        return 0;
>> +    }
>> +
>> +    return 1;
>> +}
>> +

> 
> Don:  Feel free to retain my Reviewed-by, subject to the adjustments listed.
> 

Ok,
  Thanks.

   -Don Slutz

> ~Andrew
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v10 02/10] xen: Add support for VMware cpuid leaves
  2015-05-20  8:03     ` Julien Grall
@ 2015-05-20 18:14       ` Don Slutz
  0 siblings, 0 replies; 22+ messages in thread
From: Don Slutz @ 2015-05-20 18:14 UTC (permalink / raw)
  To: Julien Grall, Andrew Cooper, xen-devel
  Cc: Kevin Tian, Keir Fraser, Ian Campbell, Stefano Stabellini,
	George Dunlap, Ian Jackson, Eddie Dong, Tim Deegan, Jan Beulich,
	Aravind Gopalakrishnan, Jun Nakajima, Boris Ostrovsky,
	Suravee Suthikulpanit

On 05/20/15 04:03, Julien Grall wrote:
> Hi,
> 
> On 19/05/2015 21:02, Andrew Cooper wrote:
>>> diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
>>> index cea3fe7..5a5bad6 100644
>>> --- a/xen/include/public/arch-x86/xen.h
>>> +++ b/xen/include/public/arch-x86/xen.h
>>> @@ -263,7 +263,7 @@ struct arch_shared_info {
>>>   typedef struct arch_shared_info arch_shared_info_t;
>>>
>>>   struct xen_arch_domainconfig {
>>> -    char dummy;
>>> +    uint64_t vmware_hwver;
>>
>> Julien: (as the author of the introducing patch) Given that this
>> structure is used as part of a domain_create hypercall only, I think a
>> comment is warrented stating that its API/ABI is covered by the
>> DOMCTL_INTERFACE_VERSION.
> 
> It's part of DOMCTL_INTERFACE_VERSION. Feel free to send a patch to add 
> a comment.
> 
>> Strictly speaking, this is currently part of the public ABI which must
>> always maintain backwards compatibility.
> 
> I don't think it's necessary to maintain backwards compatibility.  This 
> has been added during this is release cycle.
> 
>> Don:  Feel free to retain my Reviewed-by, subject to the adjustments listed.
> 
> It's rather strange to have a separate patch for adding vmware_hwver 
> version in the tools.

That was done to reduce the size of the patches.

> You are relying on the fact that the toolstack 
> memset zero xen_arch_domainconfig to zero which is hidden in the maze of 
> the code (the memset is done on an upper container).
> 
> It would be worth to add an explicit vmware_hwver = 0 in the 
> libxl__arch_domain_prepare_config.
> 

Ok, I will do so (the next patch does change this code).  This does add
a requirement of a tools ack to this patch.

   -Don Slutz

> Regards,
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v10 01/10] tools: Add vga=vmware
  2015-05-20 17:40       ` Don Slutz
@ 2015-05-20 22:52         ` Andrew Cooper
  0 siblings, 0 replies; 22+ messages in thread
From: Andrew Cooper @ 2015-05-20 22:52 UTC (permalink / raw)
  To: xen-devel

On 20/05/2015 18:40, Don Slutz wrote:
> On 05/15/15 04:49, Ian Campbell wrote:
>> On Fri, 2015-05-15 at 00:42 +0100, Andrew Cooper wrote:
>>> On 15/05/2015 00:34, Don Slutz wrote:
>>>> This allows use of QEMU's VMware emulated video card
>>>>
>>>> Signed-off-by: Don Slutz <dslutz@verizon.com>
>>> Nack.
>>>
>>> Qemu-trad is currently has remote code execution vulnerabilities in its
>>> vmware vga model.  CVE-2014-3689 amongst others.
>> Maybe we should only be exposing this new functionality with the
>> qemu-upstream model?
>>
>> In general we've not been taking new development to -trad for some time.
>>
> I plan to go with the prevent usage of vga=vmware in
> device_model_version=qemu-xen-traditional
>
>    -Don Slutz

That is perfectly fine from my point of view.  (All I care about is not
exposing known RCEs)

~Andrew

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2015-05-20 22:52 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-14 23:34 [PATCH v10 00/10] Xen VMware tools support Don Slutz
2015-05-14 23:34 ` [PATCH v10 01/10] tools: Add vga=vmware Don Slutz
2015-05-14 23:42   ` Andrew Cooper
2015-05-14 23:55     ` Don Slutz
2015-05-15  8:49     ` Ian Campbell
2015-05-20 17:40       ` Don Slutz
2015-05-20 22:52         ` Andrew Cooper
2015-05-14 23:34 ` [PATCH v10 02/10] xen: Add support for VMware cpuid leaves Don Slutz
2015-05-19 20:02   ` Andrew Cooper
2015-05-20  8:03     ` Julien Grall
2015-05-20 18:14       ` Don Slutz
2015-05-20 17:48     ` Don Slutz
2015-05-14 23:34 ` [PATCH v10 03/10] tools: Add vmware_hwver support Don Slutz
2015-05-14 23:34 ` [PATCH v10 04/10] vmware: Add VMware provided include file Don Slutz
2015-05-14 23:34 ` [PATCH v10 05/10] xen: Add vmware_port support Don Slutz
2015-05-19 20:23   ` Andrew Cooper
2015-05-20 17:42     ` Don Slutz
2015-05-14 23:34 ` [PATCH v10 06/10] xen: Add ring 3 " Don Slutz
2015-05-14 23:34 ` [PATCH v10 07/10] tools: Add " Don Slutz
2015-05-14 23:34 ` [PATCH v10 08/10] Add IOREQ_TYPE_VMWARE_PORT Don Slutz
2015-05-14 23:34 ` [PATCH v10 09/10] Add xentrace to vmware_port Don Slutz
2015-05-14 23:34 ` [PATCH v10 10/10] test_x86_emulator.c: Add tests for #GP usage Don Slutz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.