[Qemu-devel] Re: [PATCH 1/1] KVM/userspace: Support for assigning PCI devices to guests

[Amit Shah <amit.shah@qumranet.com>]

Hi Anthony,

* On Thursday 28 Aug 2008 18:41:33 Anthony Liguori wrote:
> Amit Shah wrote:

> >> This patch is too big on it's on.  It should be split into logical
> >> parts.
> >
> > However, it just adds device assignment support and does nothing else. I
> > don't see a way of splitting this any more.
>
> The libkvm changes can go in there own patch.  The changes necessary to
> piix_pci can get in their own patch.  If you get creative, it's probably
> possible to split up device-assignment.c too.

Frankly, what purpose would it serve? There's only one call site for the 
functions in libkvm which are only ever going to be used by the device 
assignment code. piix_pci I can agree; they could be shared by others so it 
can be split.

> > As you noticed, it's quite different. They have a lot more code in there.
> > (3k+ lines vs 600 lines). I guess most of it went away when we realised
> > we didn't need all that. Also, I noticed most of the code there is not in
> > the qemu style.
>
> What are you looking at?  In my version of xen-unstable, I see
> pass-through.c at 694 lines.  It's all in the right style too.

I got the xen-unstable hg tree and am looking at tools/ioemu/pass-through.[ch]

> >> We try to avoid open coding calls to libkvm functions directly within
> >> QEMU.  At the least, it should be wrapped with an if (kvm_enabled()).
> >
> > With the direct-mmio patch, we just start depending on KVM. Ben-Ami, is
> > there a chance you can keep the MMIO-via-userspace method supported as
> > well for non-kvm hosts?
>
> Open-coding calls to libkvm functions breaks the build when not using
> KVM.  They all need qemu-kvm.c wrappers.

Of course, that comment's taken. I'm just wondering if Ben-Ami can redo the 
patch so that non-kvm guests can work as well.

> > From your other mail:
> >> Where did this come from originally?  It's completely different from
> >> what is in xen-unstable.  What's in xen-unstable is actually a lot nicer
> >> IMHO.
> >
> > Nicer in what way?
> >
> > I think the original code was picked up from before the time it was
> > merged in Xen.
>
> It's in the right style, it avoids layering violations (there are no
> direct calls to the piix device within the code).  At some point in

Please point me to the code you're looking at.

> time, both KVM and Xen are going to live in the upstream QEMU tree so
> this code will have to be shared.  Since we're basing our code off what
> appears to be older Xen code, I think rebasing it to the latest bits
> makes sense.

Definitely we should work together and have a common codebase. Also, the 
version I have is based on something that was pre-xen; so it's possible all 
the layering work happened later.

Let me see the version you have before I qemu-ify this code.

Thanks,
Amit