From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jan Kiszka <jan.kiszka@siemens.com>
Subject: Re: [PATCH v2 4/4] kvm: i386: Add classic PCI device assignment
Date: Wed, 29 Aug 2012 10:44:38 +0200
Message-ID: <503DD676.80907@siemens.com>
References: <cover.1346048917.git.jan.kiszka@web.de> <825e653c9cfe9d8e26185917cbe1f1dd7ae299e2.1346048917.git.jan.kiszka@web.de> <503CA542.7030707@siemens.com> <20120828214912.GE5817@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Cc: Avi Kivity <avi@redhat.com>, Marcelo Tosatti <mtosatti@redhat.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	Alex Williamson <alex.williamson@redhat.com>,
	Blue Swirl <blauwirbel@gmail.com>,
	=?ISO-8859-1?Q?Andreas_F=E4rber?= <afaerber@suse.de>
To: "Michael S. Tsirkin" <mst@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from goliath.siemens.de ([192.35.17.28]:29533 "EHLO
	goliath.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752868Ab2H2Iot (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 29 Aug 2012 04:44:49 -0400
In-Reply-To: <20120828214912.GE5817@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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 <jan.kiszka@siemens.com>
>> ---
>>
>> 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