From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41229) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X1nhN-0005GV-DS for qemu-devel@nongnu.org; Mon, 30 Jun 2014 22:21:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X1nhI-0004Wd-Ea for qemu-devel@nongnu.org; Mon, 30 Jun 2014 22:21:33 -0400 Received: from mga02.intel.com ([134.134.136.20]:37834) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X1nhI-0004WX-5E for qemu-devel@nongnu.org; Mon, 30 Jun 2014 22:21:28 -0400 Message-ID: <53B21B0E.7020201@intel.com> Date: Tue, 01 Jul 2014 10:21:02 +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> <53AD1BA1.9060107@intel.com> In-Reply-To: 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: Stefano Stabellini Cc: peter.maydell@linaro.org, xen-devel@lists.xensource.com, "Michael S. Tsirkin" , allen.m.kay@intel.com, Kelly.Zytaruk@amd.com, qemu-devel@nongnu.org, yang.z.zhang@intel.com, anthony@codemonkey.ws, anthony.perard@citrix.com, pbonzini@redhat.com On 2014/7/1 3:34, Stefano Stabellini wrote: > On Fri, 27 Jun 2014, Chen, Tiejun wrote: >> 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. > > The Xen PT code came first. At the time Anthony Liguori and I argued for > sharing the PT code with KVM going forward but Avi wanted to retain the > KVM PT code as is in order not to introduce any regressions on KVM. > > Ah.. the good old times :-) Thanks for your information. So now what's next I should do now? Thanks Tiejun > > http://lists.xen.org/archives/html/xen-devel/2011-10/msg00195.html > >