Re: [PATCH v12 0/8] Xen VMware tools support

From: "Jan Beulich" <JBeulich@suse.com>
To: Wei Liu <wei.liu2@citrix.com>, Don Slutz <don.slutz@gmail.com>
Cc: Jun Nakajima <jun.nakajima@intel.com>, Tim Deegan <tim@xen.org>,
	Kevin Tian <kevin.tian@intel.com>, Keir Fraser <keir@xen.org>,
	Ian Campbell <ian.campbell@citrix.com>,
	Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	xen-devel@lists.xen.org, Eddie Dong <eddie.dong@intel.com>,
	Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>,
	Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>
Subject: Re: [PATCH v12 0/8] Xen VMware tools support
Date: Fri, 10 Jul 2015 09:40:59 +0100	[thread overview]
Message-ID: <559FA13B020000780008F41F@mail.emea.novell.com> (raw)
In-Reply-To: <1435447665-5433-1-git-send-email-Don.Slutz@Gmail.com>

>>> On 28.06.15 at 01:27, <don.slutz@gmail.com> wrote:
> Changes v11 to v12:
>   Rebased on staging.
> 
>   Jan Beulich (xen: Add ring 3 vmware_port support)
>     As there don't seem to be enough convincing arguments for this
>     to be worthwhile, I'm going to drop this...
>       Dropped the 2 ring3 patches.
> 
>   Ian Campbell (tools: Add vmware_hwver support):
>     s/come/comes/
>       Done
>     I'm not sure this hunk has anything to do with this patch, nor
>     what the semantic difference between the old and new text is
>     supposed to be.
>       Dropped comment change.
> 
>   Ian Campbell (multiple):
>     In v11 this seems to have morphed into only
>     LIBXL_HAVE_LIBXL_VGA_INTERFACE_TYPE_VMWARE being provided, which
>     is clearly not an appropriate umbrella #define.
> 
>     "#define LIBXL_HAVE_CREATEINFO_VMWARE 1"
>     Lets just have a single one of these indicating support for
>     vmware, it should be added at the end of the series after all
>     the baseline vmware functionality is in place. I think that
>     means hwver, vga=vmware and this port stuff.
> 
>       Make (tools: Add vga=vmware) no longer independent.
>       Change the #define to "LIBXL_HAVE_VMWARE"
> 
>    Jan Beulich (xen: Add vmware_port support):
>      Surrounding code avoiding the use of "break" makes the result
>      look rather inconsistent. Please move this up immediately after
>      the XSM check, or drop the "break".
>        Moved it up.
> 
>    George Dunlap (Add IOREQ_TYPE_VMWARE_PORT):
>      Sorry for coming a bit late to this party.  On a high level I
>      think this is good, but there doesn't seem to be anything in
>      here in particular that is vmware-specific.  Would it make more
>      sense to give this a more generic name, and have it include all
>      of the general-purpose registers?
>        Did not change at this time because QEMU would need changes 1st.
> 
>    George Dunlap (Add xentrace to vmware_port):
>      Do you need to log edi as well? It looks like it's not used.
>        I think it makes sense to indicate that edi can be used and
>        may in the future.
>      And do you need to log all the registers here?  It seems like
>      port + regs->_ecx would be enough to tell you why it got
>      ignored.
>         Adjusted to log just port and eax (ecx was NOT tested for).

I was actually meaning to apply this (at least until patch 6; I'm
not convinced patch 7 makes sense to go in at this point), but
noticed that comments by Konrad were still un-addressed. I'm
also expecting some re-basing to be needed on top of Paul's
emulation re-work.

Wei, considering that this had already missed 4.5, would you
be okay to take at least first 6 patches (which seem reasonably
risk free) after the freeze?

Jan