From mboxrd@z Thu Jan 1 00:00:00 1970 From: Don Slutz Subject: Re: [RFC][PATCH v2x prototype 1/1] Add IOREQ_TYPE_VMWARE_PORT Date: Mon, 13 Oct 2014 13:11:19 -0400 Message-ID: <543C07B7.2020207@terremark.com> References: <542EFF81.20407@terremark.com> <1412864775-1183-1-git-send-email-dslutz@verizon.com> <9AAE0902D5BC7E449B7C8E4E778ABCD0110FFE35@AMSPEX01CL01.citrite.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <9AAE0902D5BC7E449B7C8E4E778ABCD0110FFE35@AMSPEX01CL01.citrite.net> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Paul Durrant , Don Slutz , "xen-devel@lists.xen.org" Cc: "Keir (Xen.org)" , Ian Campbell , Jan Beulich List-Id: xen-devel@lists.xenproject.org On 10/13/14 09:26, Paul Durrant wrote: >> -----Original Message----- >> From: Don Slutz [mailto:dslutz@verizon.com] >> Sent: 09 October 2014 15:26 >> To: xen-devel@lists.xen.org; Paul Durrant >> Cc: Jan Beulich; Keir (Xen.org); Ian Campbell; Don Slutz >> Subject: [RFC][PATCH v2x prototype 1/1] Add IOREQ_TYPE_VMWARE_PORT >> >> This adds synchronisation 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 >> >> QEMU patch is named "xen-hvm.c: Add support for Xen access to vmport" >> >> Signed-off-by: Don Slutz >> --- >> As requested by Paul Durrant >> >> Here is a prototype of the QEMU change using a 2nd shared page. >> I picked adding HVM_PARAM_VMPORT_IOREQ_PFN as the simple and >> fast way to handle QEMU building on older Xen versions. >> >> There is xentrace and debug logging that is TBD for the Xen 4.6 >> submission of this. ... >> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c >> index c0f47d2..4b8ea8f 100644 >> --- a/xen/arch/x86/hvm/emulate.c >> +++ b/xen/arch/x86/hvm/emulate.c >> @@ -52,12 +52,14 @@ static void hvmtrace_io_assist(int is_mmio, ioreq_t >> *p) >> >> static int hvmemul_do_io( >> int is_mmio, paddr_t addr, unsigned long *reps, int size, >> - paddr_t ram_gpa, int dir, int df, void *p_data) >> + paddr_t ram_gpa, int dir, int df, void *p_data, >> + struct cpu_user_regs *regs) >> { >> struct vcpu *curr = current; >> struct hvm_vcpu_io *vio; >> ioreq_t p = { >> - .type = is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO, >> + .type = regs ? IOREQ_TYPE_VMWARE_PORT : >> + is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO, >> .addr = addr, >> .size = size, >> .dir = dir, >> @@ -65,11 +67,15 @@ static int hvmemul_do_io( >> .data = ram_gpa, >> .data_is_ptr = (p_data == NULL), >> }; >> + vmware_ioreq_t vp; >> + vmware_ioreq_t *vpp; >> unsigned long ram_gfn = paddr_to_pfn(ram_gpa); >> p2m_type_t p2mt; >> struct page_info *ram_page; >> int rc; >> >> + BUILD_BUG_ON(sizeof(ioreq_t) < sizeof(vmware_ioreq_t)); >> + >> /* Check for paged out page */ >> ram_page = get_page_from_gfn(curr->domain, ram_gfn, &p2mt, >> P2M_UNSHARE); >> if ( p2m_is_paging(p2mt) ) >> @@ -101,7 +107,17 @@ static int hvmemul_do_io( >> return X86EMUL_UNHANDLEABLE; >> } >> >> - if ( !p.data_is_ptr && (dir == IOREQ_WRITE) ) >> + if ( regs ) >> + { >> + vpp = &vp; >> + p.data = regs->rax; >> + vp.ebx = regs->rbx; >> + vp.ecx = regs->rcx; >> + vp.edx = regs->rdx; >> + vp.esi = regs->rsi; >> + vp.edi = regs->rdi; >> + } >> + else if ( !p.data_is_ptr && (dir == IOREQ_WRITE) ) >> { >> memcpy(&p.data, p_data, size); >> p_data = NULL; >> @@ -161,7 +177,19 @@ static int hvmemul_do_io( >> put_page(ram_page); >> return X86EMUL_RETRY; >> } >> + case HVMIO_awaiting_completion: >> + if ( regs ) >> + { >> + /* May have to wait for previous cycle of a multi-write to complete. >> */ >> + if ( vio->mmio_retry ) { >> + gdprintk(XENLOG_WARNING, "WARNING: mmio_retry >> io_state=%d?\n", vio->io_state); >> + return X86EMUL_RETRY; >> + } >> + /* These are expected if we get here via hvmemul_do_io() */ >> + break; >> + } >> default: >> + gdprintk(XENLOG_WARNING, "WARNING: io_state=%d?\n", vio- >>> io_state); >> if ( ram_page ) >> put_page(ram_page); >> return X86EMUL_UNHANDLEABLE; >> @@ -175,7 +203,7 @@ static int hvmemul_do_io( >> return X86EMUL_UNHANDLEABLE; >> } >> >> - vio->io_state = >> + vio->io_state = regs ? HVMIO_handle_vmport_awaiting_completion : >> (p_data == NULL) ? HVMIO_dispatched : HVMIO_awaiting_completion; >> vio->io_size = size; >> >> @@ -197,6 +225,9 @@ static int hvmemul_do_io( >> if ( rc == X86EMUL_UNHANDLEABLE ) >> rc = hvm_buffered_io_intercept(&p); >> } >> + else if ( regs ) { >> + rc = X86EMUL_UNHANDLEABLE; >> + } >> else >> { >> rc = hvm_portio_intercept(&p); >> @@ -210,7 +241,7 @@ static int hvmemul_do_io( >> p.state = STATE_IORESP_READY; >> if ( !vio->mmio_retry ) >> { >> - hvm_io_assist(&p); >> + hvm_io_assist(&p, vpp); >> vio->io_state = HVMIO_none; >> } >> else >> @@ -226,13 +257,19 @@ static int hvmemul_do_io( >> } >> else >> { >> - rc = X86EMUL_RETRY; >> - if ( !hvm_send_assist_req(&p) ) >> + if ( regs ) >> + rc = X86EMUL_VMPORT_RETRY; >> + else >> + rc = X86EMUL_RETRY; >> + if ( !hvm_send_assist_req(&p, vpp) ) >> vio->io_state = HVMIO_none; >> else if ( p_data == NULL ) >> rc = X86EMUL_OKAY; >> } >> break; >> + case X86EMUL_VMPORT_RETRY: >> + rc = X86EMUL_RETRY; >> + break; >> default: >> BUG(); >> } > I still fail to see why you need to make such intrusive modifications to this code. I am confused. The code is doing what you say: > I would have thought you could add a new ioreq type, as you've done, but not make it 'data bearing'. I.e. you use your new VMPORT_IOREQ_PFN to carry the register values back and forth between Xen and QEMU, but you still issue a 'normal' ioreq_t structure (with your new type) via the 'normal' shared IOREQ_PFN. That way you need do nothing to the majority of the emulation code path - you'd just need to add code t copy the register values into and out of your new shared page at start and completion of I/O. I did adjust hvmemul_do_io() instead of duplicating it's code. hvmemul_do_io() cannot be used without adjustment because of the new type. I will code this up with a new routine. -Don Slutz > Paul > >