From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony Liguori Subject: Re: [PATCH 1/1] KVM/userspace: Support for assigning PCI devices to guests Date: Thu, 28 Aug 2008 08:11:33 -0500 Message-ID: <48B6A405.2060108@codemonkey.ws> References: <1219764544-29764-1-git-send-email-amit.shah@qumranet.com> <1219764544-29764-2-git-send-email-amit.shah@qumranet.com> <48B55BA2.7080409@codemonkey.ws> <200808281718.01485.amit.shah@qumranet.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Avi Kivity , kvm@vger.kernel.org, muli@il.ibm.com, benami@il.ibm.com, weidong.han@intel.com, allen.m.kay@intel.com, qemu-devel@nongnu.org, Ian Jackson To: Amit Shah Return-path: Received: from mail-gx0-f16.google.com ([209.85.217.16]:49181 "EHLO mail-gx0-f16.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752112AbYH1NMW (ORCPT ); Thu, 28 Aug 2008 09:12:22 -0400 Received: by gxk9 with SMTP id 9so4548736gxk.13 for ; Thu, 28 Aug 2008 06:12:21 -0700 (PDT) In-Reply-To: <200808281718.01485.amit.shah@qumranet.com> Sender: kvm-owner@vger.kernel.org List-ID: Amit Shah wrote: > * On Wednesday 27 Aug 2008 19:20:26 Anthony Liguori wrote: > > >>> libkvm/libkvm-x86.c | 14 + >>> libkvm/libkvm.h | 27 ++ >>> qemu/Makefile.target | 1 + >>> qemu/hw/device-assignment.c | 600 >>> +++++++++++++++++++++++++++++++++++++++++++ qemu/hw/device-assignment.h | >>> 94 +++++++ >>> qemu/hw/isa.h | 2 + >>> qemu/hw/pc.c | 9 + >>> qemu/hw/pci.c | 12 + >>> qemu/hw/pci.h | 1 + >>> qemu/hw/piix_pci.c | 19 ++ >>> qemu/vl.c | 18 ++ >>> 11 files changed, 797 insertions(+), 0 deletions(-) >>> create mode 100644 qemu/hw/device-assignment.c >>> create mode 100644 qemu/hw/device-assignment.h >>> >> 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. > 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. >> 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. > 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 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. Regards, Anthony Liguori > Amit >