From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36657) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XYloU-0004VB-9z for qemu-devel@nongnu.org; Mon, 29 Sep 2014 21:01:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XYloO-0005ZC-66 for qemu-devel@nongnu.org; Mon, 29 Sep 2014 21:01:10 -0400 Received: from omzsmtpe04.verizonbusiness.com ([199.249.25.207]:52521) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XYloO-0005Xz-2D for qemu-devel@nongnu.org; Mon, 29 Sep 2014 21:01:04 -0400 From: Don Slutz Message-ID: <542A00C7.2030509@terremark.com> Date: Mon, 29 Sep 2014 21:00:55 -0400 MIME-Version: 1.0 References: <1411757235-29128-1-git-send-email-dslutz@verizon.com> <1411757235-29128-2-git-send-email-dslutz@verizon.com> <54291463.1080205@suse.de> <54293E24.3080205@redhat.com> <54294832.9060001@suse.de> In-Reply-To: <54294832.9060001@suse.de> 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: Alexander Graf , Paolo Bonzini , Don Slutz , qemu-devel@nongnu.org Cc: xen-devel@lists.xensource.com, Marcel Apfelbaum , "Michael S. Tsirkin" , Markus Armbruster , Anthony Liguori , =?windows-1252?Q?Andreas_F=E4rber?= , Stefano Stabellini On 09/29/14 07:53, Alexander Graf wrote: > > On 29.09.14 13:10, Paolo Bonzini wrote: >> Il 29/09/2014 10:12, Alexander Graf ha scritto: >>> Could you instead plug into the existing cpu_synchronize_registers() >>> framework and just implement register synchronization for the Xen side, >>> just like it's been done for KVM? :) >> No, because here it's Xen that sends out the register contents. With >> KVM, it's QEMU that requests the register contents. > So? We could still reuse the same infrastructure: > > > cpu_handle_ioreq() > { > ... > > if (req->type == IOPORT_REGISTERS_SYNCHRONIZED) { > cpu->xen_vcpu_dirty = true; I am assuming that you mean something like: X86CPU *cpu = X86_CPU(current_cpu); cpu->xen_vcpu_dirty = true; because cpu is not defined here. And this has a major issue because current_cpu is NULL for Xen, so you have added a SIGSEGV. If you are trying to say "lets build a CPUState state object here and set current_cpu to it, and then set the xen_vcpu_dirty" then yes that could be done. Adding all the code to build a CPUState state object and X86CPUClass object correctly just to use the code in vmport.c looked to me to be a lot of work for little benefit. If you want to go with not adding the objects, I could change to having a "xen X86CPU" that I set current_cpu to and then back to NULL when done. That (I think) would drop all changes to vmport.c > synchronize_xen_to_env(xenptr, cpu); > } > > handle_ioreq(); > > if (req->type == IOPORT_REGISTERS_SYNCHRONIZED) { > cpu->xen_vcpu_dirty = false; > synchronize_env_to_xen(xenptr, cpu); > } > > ... > } > > void xen_cpu_synchronize_state(CPUState *cpu) > { > assert(cpu->xen_vcpu_dirty); > } > > Then no changes to the vmport code would be necessary and this problems > where some code path wants to do direct access to registers > automatically tells us that things are broken. The SIGSEGV already does this. -Don Slutz > > Alex 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 21:00:55 -0400 Message-ID: <542A00C7.2030509@terremark.com> References: <1411757235-29128-1-git-send-email-dslutz@verizon.com> <1411757235-29128-2-git-send-email-dslutz@verizon.com> <54291463.1080205@suse.de> <54293E24.3080205@redhat.com> <54294832.9060001@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <54294832.9060001@suse.de> 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: Alexander Graf , Paolo Bonzini , Don Slutz , qemu-devel@nongnu.org Cc: xen-devel@lists.xensource.com, Marcel Apfelbaum , "Michael S. Tsirkin" , Markus Armbruster , Anthony Liguori , =?windows-1252?Q?Andreas_F=E4rber?= , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On 09/29/14 07:53, Alexander Graf wrote: > > On 29.09.14 13:10, Paolo Bonzini wrote: >> Il 29/09/2014 10:12, Alexander Graf ha scritto: >>> Could you instead plug into the existing cpu_synchronize_registers() >>> framework and just implement register synchronization for the Xen side, >>> just like it's been done for KVM? :) >> No, because here it's Xen that sends out the register contents. With >> KVM, it's QEMU that requests the register contents. > So? We could still reuse the same infrastructure: > > > cpu_handle_ioreq() > { > ... > > if (req->type == IOPORT_REGISTERS_SYNCHRONIZED) { > cpu->xen_vcpu_dirty = true; I am assuming that you mean something like: X86CPU *cpu = X86_CPU(current_cpu); cpu->xen_vcpu_dirty = true; because cpu is not defined here. And this has a major issue because current_cpu is NULL for Xen, so you have added a SIGSEGV. If you are trying to say "lets build a CPUState state object here and set current_cpu to it, and then set the xen_vcpu_dirty" then yes that could be done. Adding all the code to build a CPUState state object and X86CPUClass object correctly just to use the code in vmport.c looked to me to be a lot of work for little benefit. If you want to go with not adding the objects, I could change to having a "xen X86CPU" that I set current_cpu to and then back to NULL when done. That (I think) would drop all changes to vmport.c > synchronize_xen_to_env(xenptr, cpu); > } > > handle_ioreq(); > > if (req->type == IOPORT_REGISTERS_SYNCHRONIZED) { > cpu->xen_vcpu_dirty = false; > synchronize_env_to_xen(xenptr, cpu); > } > > ... > } > > void xen_cpu_synchronize_state(CPUState *cpu) > { > assert(cpu->xen_vcpu_dirty); > } > > Then no changes to the vmport code would be necessary and this problems > where some code path wants to do direct access to registers > automatically tells us that things are broken. The SIGSEGV already does this. -Don Slutz > > Alex