From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47376) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X0QUf-0001VZ-Rm for qemu-devel@nongnu.org; Fri, 27 Jun 2014 03:22:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X0QUZ-0004QR-Jk for qemu-devel@nongnu.org; Fri, 27 Jun 2014 03:22:45 -0400 Received: from mga02.intel.com ([134.134.136.20]:1681) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X0QUZ-0004QL-EM for qemu-devel@nongnu.org; Fri, 27 Jun 2014 03:22:39 -0400 Message-ID: <53AD1BA1.9060107@intel.com> Date: Fri, 27 Jun 2014 15:22:09 +0800 From: "Chen, Tiejun" MIME-Version: 1.0 References: <20140625064545.GB25563@redhat.com> <53AA8404.8040708@intel.com> <20140625082850.GB32652@redhat.com> <53AA8AAB.30100@intel.com> <20140625084347.GE32652@redhat.com> <53AA8CC2.7090406@intel.com> <20140625090420.GG32652@redhat.com> <53AA92F6.1090803@intel.com> <20140625092141.GL32652@redhat.com> <53AA9650.1060503@intel.com> <20140625094455.GB6357@redhat.com> <53AA9D53.4010407@intel.com> In-Reply-To: <53AA9D53.4010407@intel.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [v5][PATCH 2/5] xen, gfx passthrough: create pseudo intel isa bridge List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: peter.maydell@linaro.org, xen-devel@lists.xensource.com, stefano.stabellini@eu.citrix.com, allen.m.kay@intel.com, Kelly.Zytaruk@amd.com, qemu-devel@nongnu.org, anthony.perard@citrix.com, anthony@codemonkey.ws, yang.z.zhang@intel.com, pbonzini@redhat.com On 2014/6/25 17:58, Chen, Tiejun wrote: > On 2014/6/25 17:44, Michael S. Tsirkin wrote: >> On Wed, Jun 25, 2014 at 05:28:48PM +0800, Chen, Tiejun wrote: >>> On 2014/6/25 17:21, Michael S. Tsirkin wrote: >>>> On Wed, Jun 25, 2014 at 05:14:30PM +0800, Chen, Tiejun wrote: >>>>> On 2014/6/25 17:04, Michael S. Tsirkin wrote: >>>>>> On Wed, Jun 25, 2014 at 04:48:02PM +0800, Chen, Tiejun wrote: >>>>>>> On 2014/6/25 16:43, Michael S. Tsirkin wrote: >>>>>>>> On Wed, Jun 25, 2014 at 04:39:07PM +0800, Chen, Tiejun wrote: >>>>>>>>>> In fact it's exactly what passthrough does. >>>>>>>>>> I wonder if more bits from ./hw/i386/kvm/pci-assign.c >>>>>>>>>> can be reused. How do you poke at the host device? sysfs? >>>>>>>>> >>>>>>>>> Yes, sysfs. >>>>>>>>> >>>>>>>>> Thanks >>>>>>>>> Tiejun >>>>>>>> >>>>>>>> Then you should be able to re-use large chunks of >>>>>>>> ./hw/i386/kvm/pci-assign.c: basically everything >>>>>>>> that deals with emulation. >>>>>>> >>>>>>> Do you mean those hooks to get info from the real device? Xen >>>>>>> have its own >>>>>>> wrapper, xen_host_pci_get_block(), so we always go there in xen >>>>>>> scenario. >>>>>>> >>>>>>> Thanks >>>>>>> Tiejun >>>>>> >>>>>> Yes and that's not good. We have two pieces of code doing mostly >>>>>> identical things slightly differently. >>>>>> hw/i386/kvm/pci-assign.c is a bit younger so it's cleaner, >>>>>> but these really need to be unified. >>>>>> >>>>> >>>>> Sorry, take a look at this again, >>>>> >>>>> xen_host_pci_get_block(XenHostPCIDevice *d, int pos, uint8_t *buf, >>>>> int len) >>>>> | >>>>> + xen_host_pci_config_read(d, pos, buf, len) >>>>> | >>>>> + pread(d->config_fd, buf, len, pos) >>>>> >>>>> I thinks this should be same as kvm. >>>>> >>>>> Thanks >>>>> Tiejun >>>> >>>> get_block is trivial. >>>> >>>> I really mean the whole PT infrastructure for >>>> - discovering host devices through sysfs >>>> - virtualizing devices >>>> >>>> rom, bars, msi ... >>>> the list goes on. >>>> >>>> logic is mostly the same. >>>> >>> >>> Looks you mean we can unify the entire PT infrastructure between kvm >>> and xen >>> inside qemu. But I'm afraid its not easy to do in a short time, so >>> maybe we >>> can queue this as next phase. >>> >>> Thanks >>> Tiejun >> >> I'm afraid once we merge your code, you'll lose interest :) >> > > Currently we have to push this feature into upstream as our first > priority, so unless something is really needed to address. Of course I > hope this point what we're talking is not such a thing :) > > But I can promise here I'd like to do this optimization with your guide > next :) > >> At least, don't add duplicate code for ROM. >> > > Let me try this. > Its not easy as expected. kvm always work with this structure, AssignedDevice, and especially this is just activated in kvm_enabled(). And then set all properties to this structure. In xen case, the similar structure, XenHostPCIDevice, is not easy transferred into the structure, AssignedDevice. So this mean we have to split assigned_dev_load_option_rom() as line by line for xen and kvm, respectively. I really agree we definitely need to unify PT infrastructure between kvm and xen after this try since I can't understand why we originally introduce same way to do same thing :( Do you have better idea? If not, I prefer we open this completely as next action to follow-up. But this time I'm afraid I can't get in this. Thanks Tiejun From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: [v5][PATCH 2/5] xen, gfx passthrough: create pseudo intel isa bridge Date: Fri, 27 Jun 2014 15:22:09 +0800 Message-ID: <53AD1BA1.9060107@intel.com> References: <20140625064545.GB25563@redhat.com> <53AA8404.8040708@intel.com> <20140625082850.GB32652@redhat.com> <53AA8AAB.30100@intel.com> <20140625084347.GE32652@redhat.com> <53AA8CC2.7090406@intel.com> <20140625090420.GG32652@redhat.com> <53AA92F6.1090803@intel.com> <20140625092141.GL32652@redhat.com> <53AA9650.1060503@intel.com> <20140625094455.GB6357@redhat.com> <53AA9D53.4010407@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <53AA9D53.4010407@intel.com> 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: "Michael S. Tsirkin" Cc: peter.maydell@linaro.org, xen-devel@lists.xensource.com, stefano.stabellini@eu.citrix.com, allen.m.kay@intel.com, Kelly.Zytaruk@amd.com, qemu-devel@nongnu.org, anthony.perard@citrix.com, anthony@codemonkey.ws, yang.z.zhang@intel.com, pbonzini@redhat.com List-Id: xen-devel@lists.xenproject.org On 2014/6/25 17:58, Chen, Tiejun wrote: > On 2014/6/25 17:44, Michael S. Tsirkin wrote: >> On Wed, Jun 25, 2014 at 05:28:48PM +0800, Chen, Tiejun wrote: >>> On 2014/6/25 17:21, Michael S. Tsirkin wrote: >>>> On Wed, Jun 25, 2014 at 05:14:30PM +0800, Chen, Tiejun wrote: >>>>> On 2014/6/25 17:04, Michael S. Tsirkin wrote: >>>>>> On Wed, Jun 25, 2014 at 04:48:02PM +0800, Chen, Tiejun wrote: >>>>>>> On 2014/6/25 16:43, Michael S. Tsirkin wrote: >>>>>>>> On Wed, Jun 25, 2014 at 04:39:07PM +0800, Chen, Tiejun wrote: >>>>>>>>>> In fact it's exactly what passthrough does. >>>>>>>>>> I wonder if more bits from ./hw/i386/kvm/pci-assign.c >>>>>>>>>> can be reused. How do you poke at the host device? sysfs? >>>>>>>>> >>>>>>>>> Yes, sysfs. >>>>>>>>> >>>>>>>>> Thanks >>>>>>>>> Tiejun >>>>>>>> >>>>>>>> Then you should be able to re-use large chunks of >>>>>>>> ./hw/i386/kvm/pci-assign.c: basically everything >>>>>>>> that deals with emulation. >>>>>>> >>>>>>> Do you mean those hooks to get info from the real device? Xen >>>>>>> have its own >>>>>>> wrapper, xen_host_pci_get_block(), so we always go there in xen >>>>>>> scenario. >>>>>>> >>>>>>> Thanks >>>>>>> Tiejun >>>>>> >>>>>> Yes and that's not good. We have two pieces of code doing mostly >>>>>> identical things slightly differently. >>>>>> hw/i386/kvm/pci-assign.c is a bit younger so it's cleaner, >>>>>> but these really need to be unified. >>>>>> >>>>> >>>>> Sorry, take a look at this again, >>>>> >>>>> xen_host_pci_get_block(XenHostPCIDevice *d, int pos, uint8_t *buf, >>>>> int len) >>>>> | >>>>> + xen_host_pci_config_read(d, pos, buf, len) >>>>> | >>>>> + pread(d->config_fd, buf, len, pos) >>>>> >>>>> I thinks this should be same as kvm. >>>>> >>>>> Thanks >>>>> Tiejun >>>> >>>> get_block is trivial. >>>> >>>> I really mean the whole PT infrastructure for >>>> - discovering host devices through sysfs >>>> - virtualizing devices >>>> >>>> rom, bars, msi ... >>>> the list goes on. >>>> >>>> logic is mostly the same. >>>> >>> >>> Looks you mean we can unify the entire PT infrastructure between kvm >>> and xen >>> inside qemu. But I'm afraid its not easy to do in a short time, so >>> maybe we >>> can queue this as next phase. >>> >>> Thanks >>> Tiejun >> >> I'm afraid once we merge your code, you'll lose interest :) >> > > Currently we have to push this feature into upstream as our first > priority, so unless something is really needed to address. Of course I > hope this point what we're talking is not such a thing :) > > But I can promise here I'd like to do this optimization with your guide > next :) > >> At least, don't add duplicate code for ROM. >> > > Let me try this. > Its not easy as expected. kvm always work with this structure, AssignedDevice, and especially this is just activated in kvm_enabled(). And then set all properties to this structure. In xen case, the similar structure, XenHostPCIDevice, is not easy transferred into the structure, AssignedDevice. So this mean we have to split assigned_dev_load_option_rom() as line by line for xen and kvm, respectively. I really agree we definitely need to unify PT infrastructure between kvm and xen after this try since I can't understand why we originally introduce same way to do same thing :( Do you have better idea? If not, I prefer we open this completely as next action to follow-up. But this time I'm afraid I can't get in this. Thanks Tiejun