Re: [PATCH for-4.5 v6 00/16] Xen VMware tools support

[Don Slutz <dslutz@verizon.com>]

On 09/24/14 11:52, George Dunlap wrote:
> On 09/23/2014 01:30 PM, Ian Campbell wrote:
>> On Mon, 2014-09-22 at 13:19 -0400, Don Slutz wrote:
>>>>> It sounds plausible, for sure.
>>>>>
>>>>> Even so, why can't the result of that #GP be a calldown into qemu for
>>>>> further processing?
>>> This is not simple in that QEMU does not have access to the VCPU
>>> registers.  Unlike a normal
>>> I/O request, vmware_port (aka vmport) both reads and writes VCPU 
>>> registers.
>> Are you saying that emulating a normal in or out instruction doesn't
>> require accessing vcpu registers? Are you sure? Surely it needs to
>> either read the source or write the destination register somehow.
>>
>>>> I was only responding to the part of your comment in parentheses. :-)
>>>>
>>>> I suppose in large part it would depend on what the hypercalls were
>>>> actually doing; I'd have to go back and look at them to say if they
>>>> need to be in Xen or whether they could be passed on to qemu.
>>>>
>>> Clearly it is possible to pass the VCPU registers to QEMU, but that is
>>> currently not done.
>> I think there's an existing hypercall to get/set the state for a vcpu,
>> perhaps it is too heavy weight to be used here though.
>>
>> An alternative would be a semantically higher level I/O req which took a
>> guest pointer to a key and a guest pointer to the buffer etc, without
>> needing the registers themselves.
>>
>>>    So a new
>>> version of QEMU would also be needed to go this way.  None the the
>>> proposed features need
>>> any data from QEMU, so I do not think this make sense.
>> The concern is that it is adding a load of complex looking string and
>> pointer manipulation stuff to the hypervisor, the sort of thing which
>> often leads to security vulnerabilities.
>
> Do you mean the instruction decoding in vmware_gp_check()?
>

I do not think so.  I think this is a reference to all the new code in

[PATCH 08/16] xen: Add limited support of VMware's hyper-call rpc


> I was wondering how hard it would be to use the generic emulation 
> code.  We already have to emulate IO instructions anyway.  This is 
> very complicated code, and having it duplicated in two places seems 
> like it's just asking for someone to update the one and forget to 
> update the other, opening up a bug / security vulnerability.
>

I did reply to some this on a different thread.  Key point being that
the current emulate IO instructions should be reporting #GP which
is not what is needed.  Also all I see is a decode and emulate. What
I need a just a decode.  The closest to just a decode is
__get_instruction_length_from_list() (an AMD only function...) which
has the issue of only returning the length of the instruction (and
not any decodeing done).  As I said in svm_vmexit_gp_intercept():


     /*
      * Just use 15 for the instruction length; vmport_gp_check will
      * adjust it.  This is because
      * __get_instruction_length_from_list() has issues, and may
      * require a double read of the instruction bytes.  At some
      * point a new routine could be added that is based on the code
      * in vmport_gp_check with extensions to make it more general.
      * Since that routine is the only user of this code this can be
      * done later.
      */

So I do not know of any code that could be shared.



> The other question would be whether doing it in qemu would be fast 
> enough, or if there would be information needed by the hypercall 
> that's not available; things like GETTIME / GETTIMEFULL / GETHZ.
>

I think it would be fast enough.  But I also do not see any need to send
the simple ones you listed to QEMU for processing.  Only the ones
that need (or could use QEMU like the RPC ones).


> On the other hand, things like GETSCREENSIZE and GETGUIOPTIONS 
> probably *are* better handled by qemu.
>

Yes.  And that includes the vmware mouse support.

    -Don Slutz

>  -George
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel