|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v9 06/13] xen: Add ring 3 vmware_port support
On 21/02/15 13:36, Don Slutz wrote: > On 02/18/15 13:19, Andrew Cooper wrote: >> On 18/02/15 17:03, Don Slutz wrote: >>> On 02/17/15 09:38, Andrew Cooper wrote: >>>> On 16/02/15 23:05, Don Slutz wrote: >>>>> 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. >>>>> >>>>> The support included is enough to allow VMware tools to install in a >>>>> HVM domU. >>>>> >>>>> 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@xxxxxxxxxxx> >>>>> --- >>>>> 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 | 62 >>>>> ++++++++++++++++++++++++++++++++-- >>>>> xen/arch/x86/hvm/svm/svm.c | 27 +++++++++++++++ >>>>> 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 | 38 +++++++++++++++++++++ >>>>> xen/arch/x86/x86_emulate/x86_emulate.c | 25 +++++++++++--- >>>>> xen/arch/x86/x86_emulate/x86_emulate.h | 8 +++++ >>>>> xen/include/asm-x86/hvm/emulate.h | 2 ++ >>>>> xen/include/asm-x86/hvm/vmport.h | 1 + >>>>> 10 files changed, 172 insertions(+), 6 deletions(-) >>>>> >>>>> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c >>>>> index 636c909..a6a6a5c 100644 >>>>> --- a/xen/arch/x86/hvm/emulate.c >>>>> +++ b/xen/arch/x86/hvm/emulate.c >>>>> @@ -22,6 +22,7 @@ >>>>> #include <asm/hvm/trace.h> >>>>> #include <asm/hvm/support.h> >>>>> #include <asm/hvm/svm/svm.h> >>>>> +#include <asm/hvm/vmport.h> >>>>> >>>>> static void hvmtrace_io_assist(int is_mmio, ioreq_t *p) >>>>> { >>>>> @@ -776,6 +777,7 @@ static int hvmemul_read_io_discard( >>>>> unsigned long *val, >>>>> struct x86_emulate_ctxt *ctxt) >>>>> { >>>>> + ctxt->do_vmport = 0; >>>> 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. >> >> Something along the lines of: >> >> diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c >> b/xen/arch/x86/x86_emulate/x86_emulate.c >> index 5e9e040..dd40d6a 100644 >> --- a/xen/arch/x86/x86_emulate/x86_emulate.c >> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c >> @@ -3394,7 +3394,8 @@ static int inject_swint(enum x86_swint_type type, >> ? insn_fetch_type(uint8_t) >> : (uint16_t)_regs.edx); >> op_bytes = !(b & 1) ? 1 : (op_bytes == 8) ? 4 : op_bytes; >> - if ( (rc = ioport_access_check(port, op_bytes, ctxt, ops)) != 0 ) >> + if ( ((rc = ioport_access_check(port, op_bytes, ctxt, ops)) != >> 0) || >> + (ops->vmport_check && ((rc = ops->vmport_check(port, >> ctxt)) != 0)) ) >> goto done; >> if ( b & 2 ) >> { >> >> would be far less invasive and AFAICT, replace the entire rest of your >> patch. >> >> In this case, if ioport_access_check() succeeds, or if it fails and >> vmport_check subsequently succeeds, the standard ioport dispatch will >> run, and hit vmport_ioport(). >> > Yes, but since vmport_ioport changes guest_cpu_user_regs() which > are different then _regs and get modified by: > > *ctxt->regs = _regs; > done: > return rc; > > which will drop all changes to other registers by vmport_ioport. This > is why I added the flag do_vmport. Which must be set in all case where > vmport_ioport is called via x86_emulate. > > So I could call on it 1st. This would make the change smaller. > > -Don Slutz > How about this then? diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c index 5e9e040..2aea91c 100644 --- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -3393,8 +3393,11 @@ static int inject_swint(enum x86_swint_type type, 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 ) { @@ -3413,6 +3416,10 @@ static int inject_swint(enum x86_swint_type type, } if ( rc != 0 ) goto done; + if ( vmport ) + { + /* fill _regs */ + } break; } ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |