From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60837) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XYlMk-0005BA-C7 for qemu-devel@nongnu.org; Mon, 29 Sep 2014 20:32:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XYlMe-0007hc-9R for qemu-devel@nongnu.org; Mon, 29 Sep 2014 20:32:30 -0400 Received: from fldsmtpe04.verizon.com ([140.108.26.143]:62503) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XYlMe-0007gm-2q for qemu-devel@nongnu.org; Mon, 29 Sep 2014 20:32:24 -0400 From: Don Slutz Message-ID: <5429FA0F.4050905@terremark.com> Date: Mon, 29 Sep 2014 20:32:15 -0400 MIME-Version: 1.0 References: <1411757235-29128-1-git-send-email-dslutz@verizon.com> <1411757235-29128-2-git-send-email-dslutz@verizon.com> In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/1] xen-hvm.c: Add support for Xen access to vmport List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefano Stabellini Cc: xen-devel@lists.xensource.com, Marcel Apfelbaum , Markus Armbruster , "Michael S. Tsirkin" , qemu-devel@nongnu.org, Don Slutz , Alexander Graf , Anthony Liguori , =?windows-1252?Q?Andreas_F=E4rber?= On 09/29/14 06:25, Stefano Stabellini wrote: > On Mon, 29 Sep 2014, Stefano Stabellini wrote: >> On Fri, 26 Sep 2014, Don Slutz wrote: >>> This adds synchronisation of the vcpu registers >>> between Xen and QEMU. >>> >>> Signed-off-by: Don Slutz >> [...] >> >>> diff --git a/xen-hvm.c b/xen-hvm.c >>> index 05e522c..e1274bb 100644 >>> --- a/xen-hvm.c >>> +++ b/xen-hvm.c >>> @@ -857,14 +857,48 @@ static void cpu_handle_ioreq(void *opaque) >>> >>> handle_buffered_iopage(state); >>> if (req) { >>> +#ifdef IOREQ_TYPE_VMWARE_PORT >> Is there any reason to #ifdef this code? >> Couldn't we just always build it in QEMU? This allows QEMU 2.2 (or later) to build on a system that has Xen 4.5 or earlier installed; and work. >> >>> + if (req->type == IOREQ_TYPE_VMWARE_PORT) { >> I think it would make more sense to check for IOREQ_TYPE_VMWARE_PORT >> from within handle_ioreq. >> Ok, I can move it. >>> + CPUX86State *env; >>> + ioreq_t fake_req = { >>> + .type = IOREQ_TYPE_PIO, >>> + .addr = (uint16_t)req->size, >>> + .size = 4, >>> + .dir = IOREQ_READ, >>> + .df = 0, >>> + .data_is_ptr = 0, >>> + }; > Why do you need a fake req? To transport the 6 VCPU registers (only 32bits of them) that vmport.c needs to do it's work. > Couldn't Xen send the real req instead? Yes, but then a 2nd exchange between QEMU and Xen would be needed to fetch the 6 VCPU registers. The ways I know of to fetch the VCPU registers from Xen, all need many cycles to do their work and return a lot of data that is not needed. The other option that I have considered was to extend the ioreq_t type to have room for these registers, but that reduces by almost half the maximum number of VCPUs that are supported (They all live on 1 page). > If any case you should spend a > few more words on why you are doing this. > Sure. Will add some form of the above as part of the commit message. >>> + if (!xen_opaque_env) { >>> + xen_opaque_env = g_malloc(sizeof(CPUX86State)); >>> + } >>> + env = xen_opaque_env; >>> + env->regs[R_EAX] = (uint32_t)(req->addr >> 32); >>> + env->regs[R_EBX] = (uint32_t)(req->addr); >>> + env->regs[R_ECX] = req->count; >>> + env->regs[R_EDX] = req->size; >>> + env->regs[R_ESI] = (uint32_t)(req->data >> 32); >>> + env->regs[R_EDI] = (uint32_t)(req->data); >>> + handle_ioreq(&fake_req); >>> + req->addr = ((uint64_t)fake_req.data << 32) | >>> + (uint32_t)env->regs[R_EBX]; >>> + req->count = env->regs[R_ECX]; >>> + req->size = env->regs[R_EDX]; >>> + req->data = ((uint64_t)env->regs[R_ESI] << 32) | >>> + (uint32_t)env->regs[R_EDI]; >> This code could be moved to a separate helper function called by >> handle_ioreq. >> Ok. -Don Slutz >>> + } else { >>> + handle_ioreq(req); >>> + } >>> +#else >>> handle_ioreq(req); >>> +#endif From mboxrd@z Thu Jan 1 00:00:00 1970 From: Don Slutz Subject: Re: [PATCH 1/1] xen-hvm.c: Add support for Xen access to vmport Date: Mon, 29 Sep 2014 20:32:15 -0400 Message-ID: <5429FA0F.4050905@terremark.com> References: <1411757235-29128-1-git-send-email-dslutz@verizon.com> <1411757235-29128-2-git-send-email-dslutz@verizon.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org Sender: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org To: Stefano Stabellini Cc: xen-devel@lists.xensource.com, Marcel Apfelbaum , Markus Armbruster , "Michael S. Tsirkin" , qemu-devel@nongnu.org, Don Slutz , Alexander Graf , Anthony Liguori , =?windows-1252?Q?Andreas_F=E4rber?= List-Id: xen-devel@lists.xenproject.org On 09/29/14 06:25, Stefano Stabellini wrote: > On Mon, 29 Sep 2014, Stefano Stabellini wrote: >> On Fri, 26 Sep 2014, Don Slutz wrote: >>> This adds synchronisation of the vcpu registers >>> between Xen and QEMU. >>> >>> Signed-off-by: Don Slutz >> [...] >> >>> diff --git a/xen-hvm.c b/xen-hvm.c >>> index 05e522c..e1274bb 100644 >>> --- a/xen-hvm.c >>> +++ b/xen-hvm.c >>> @@ -857,14 +857,48 @@ static void cpu_handle_ioreq(void *opaque) >>> >>> handle_buffered_iopage(state); >>> if (req) { >>> +#ifdef IOREQ_TYPE_VMWARE_PORT >> Is there any reason to #ifdef this code? >> Couldn't we just always build it in QEMU? This allows QEMU 2.2 (or later) to build on a system that has Xen 4.5 or earlier installed; and work. >> >>> + if (req->type == IOREQ_TYPE_VMWARE_PORT) { >> I think it would make more sense to check for IOREQ_TYPE_VMWARE_PORT >> from within handle_ioreq. >> Ok, I can move it. >>> + CPUX86State *env; >>> + ioreq_t fake_req = { >>> + .type = IOREQ_TYPE_PIO, >>> + .addr = (uint16_t)req->size, >>> + .size = 4, >>> + .dir = IOREQ_READ, >>> + .df = 0, >>> + .data_is_ptr = 0, >>> + }; > Why do you need a fake req? To transport the 6 VCPU registers (only 32bits of them) that vmport.c needs to do it's work. > Couldn't Xen send the real req instead? Yes, but then a 2nd exchange between QEMU and Xen would be needed to fetch the 6 VCPU registers. The ways I know of to fetch the VCPU registers from Xen, all need many cycles to do their work and return a lot of data that is not needed. The other option that I have considered was to extend the ioreq_t type to have room for these registers, but that reduces by almost half the maximum number of VCPUs that are supported (They all live on 1 page). > If any case you should spend a > few more words on why you are doing this. > Sure. Will add some form of the above as part of the commit message. >>> + if (!xen_opaque_env) { >>> + xen_opaque_env = g_malloc(sizeof(CPUX86State)); >>> + } >>> + env = xen_opaque_env; >>> + env->regs[R_EAX] = (uint32_t)(req->addr >> 32); >>> + env->regs[R_EBX] = (uint32_t)(req->addr); >>> + env->regs[R_ECX] = req->count; >>> + env->regs[R_EDX] = req->size; >>> + env->regs[R_ESI] = (uint32_t)(req->data >> 32); >>> + env->regs[R_EDI] = (uint32_t)(req->data); >>> + handle_ioreq(&fake_req); >>> + req->addr = ((uint64_t)fake_req.data << 32) | >>> + (uint32_t)env->regs[R_EBX]; >>> + req->count = env->regs[R_ECX]; >>> + req->size = env->regs[R_EDX]; >>> + req->data = ((uint64_t)env->regs[R_ESI] << 32) | >>> + (uint32_t)env->regs[R_EDI]; >> This code could be moved to a separate helper function called by >> handle_ioreq. >> Ok. -Don Slutz >>> + } else { >>> + handle_ioreq(req); >>> + } >>> +#else >>> handle_ioreq(req); >>> +#endif