From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:39549) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T6dtK-0004nA-ID for qemu-devel@nongnu.org; Wed, 29 Aug 2012 04:45:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T6dtE-0006ba-FM for qemu-devel@nongnu.org; Wed, 29 Aug 2012 04:44:50 -0400 Received: from goliath.siemens.de ([192.35.17.28]:34782) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T6dtE-0006Xc-5Y for qemu-devel@nongnu.org; Wed, 29 Aug 2012 04:44:44 -0400 Message-ID: <503DD676.80907@siemens.com> Date: Wed, 29 Aug 2012 10:44:38 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <825e653c9cfe9d8e26185917cbe1f1dd7ae299e2.1346048917.git.jan.kiszka@web.de> <503CA542.7030707@siemens.com> <20120828214912.GE5817@redhat.com> In-Reply-To: <20120828214912.GE5817@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 4/4] kvm: i386: Add classic PCI device assignment List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: "kvm@vger.kernel.org" , Marcelo Tosatti , "qemu-devel@nongnu.org" , Blue Swirl , Alex Williamson , Avi Kivity , =?ISO-8859-1?Q?Andreas_F=E4rber?= On 2012-08-28 23:49, Michael S. Tsirkin wrote: > On Tue, Aug 28, 2012 at 01:02:26PM +0200, Jan Kiszka wrote: >> This adds PCI device assignment for i386 targets using the classic KVM >> interfaces. This version is 100% identical to what is being maintained >> in qemu-kvm for several years and is supported by libvirt as well. It is >> expected to remain relevant for another couple of years until kernels >> without full-features and performance-wise equivalent VFIO support are >> obsolete. >> >> A refactoring to-do that should be done in-tree is to model MSI and >> MSI-X support via the generic PCI layer, similar to what VFIO is already >> doing for MSI-X. This should improve the correctness and clean up the >> code from duplicate logic. >> >> Signed-off-by: Jan Kiszka >> --- >> >> Changes in v2: >> - Addressed review comments of Andreas and most of Blue (see [1] for >> unchanged aspects) > > any chance you can address mine? Sorry, must have missed that mail. Can you point me to it? > Specifically I suggested > listing commit id that you started from. OK. > Also: > >> +static void msix_reset(AssignedDevice *dev) >> +{ >> + MSIXTableEntry *entry; >> + int i; >> + >> + if (!dev->msix_table) { >> + return; >> + } >> + >> + memset(dev->msix_table, 0, MSIX_PAGE_SIZE); >> + >> + for (i = 0, entry = dev->msix_table; i < dev->msix_max; i++, entry++) { >> + entry->ctrl = cpu_to_le32(0x1); /* Masked */ >> + } >> +} > > Is a bad name. Ideally file scope names should have a prefix > assigned_dev_ or pci_assign_ or something like that > but it's not a hard requirement. In this case > file includes msix.h so prefix msix_ is confusing. True, will fix. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux