Re: [Qemu-devel] [v5][PATCH 2/5] xen, gfx passthrough: create pseudo intel isa bridge

["Chen, Tiejun" <tiejun.chen@intel.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
>
>