From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:35941) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T6Tdg-0002G0-Je for qemu-devel@nongnu.org; Tue, 28 Aug 2012 17:48:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T6Tdf-0001TW-Cs for qemu-devel@nongnu.org; Tue, 28 Aug 2012 17:48:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:7071) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T6Tdf-0001TO-3b for qemu-devel@nongnu.org; Tue, 28 Aug 2012 17:47:59 -0400 Date: Wed, 29 Aug 2012 00:49:12 +0300 From: "Michael S. Tsirkin" Message-ID: <20120828214912.GE5817@redhat.com> References: <825e653c9cfe9d8e26185917cbe1f1dd7ae299e2.1346048917.git.jan.kiszka@web.de> <503CA542.7030707@siemens.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <503CA542.7030707@siemens.com> 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: Jan Kiszka Cc: kvm@vger.kernel.org, Marcelo Tosatti , qemu-devel@nongnu.org, Blue Swirl , Alex Williamson , Avi Kivity , Andreas =?iso-8859-1?Q?F=E4rber?= 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? Specifically I suggested listing commit id that you started from. 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. -- MST