[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

On 2014/7/1 13:47, Michael S. Tsirkin wrote:
On Mon, Jun 30, 2014 at 08:34:46PM +0100, 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.


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


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.


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.


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

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 :-)


Right, I remember that there was an idea to first make the code
reusable before merging either Xen or KVM PT, but in the end
the plan to merge both and then abstract it out, won.
We never got to abstracting it out but we should!

Yet, I do not think we should require abstracting all of Xen PT as
a pre-requisite for this work, but I think we can
avoid adding to the duplication without a lot of effort.
To start sharing with ROM code, I think we should rename
AssignedDevice->KvmAssignedDevice, and move fields that can be shared
with Xen from there to AssignedDevice.

Its not simple to share something between AssignedDevice and the similar structure, XenHostPCIDevice.

As I said previously AssignedDevice is just activated in kvm_enabled(), then set all properties to this structure.

But kvm_enabled() is zero since we always disable this in xen scenario. So this means we have to invoke pci-assign framework completely if we try to force to use this. So actually this would be equal to merge kvm and xen.

Or if you already figure out to avoid this happened let me know.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.