From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [PATCH 0/4 v12] MSI-X MMIO support for KVM Date: Wed, 2 Mar 2011 15:51:20 -0300 Message-ID: <20110302185120.GB21289@amt.cnet> References: <1299050817-30792-1-git-send-email-sheng@linux.intel.com> <20110302092314.GB27437@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Sheng Yang , Avi Kivity , Alex Williamson , kvm@vger.kernel.org To: "Michael S. Tsirkin" Return-path: Received: from mx1.redhat.com ([209.132.183.28]:3030 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753984Ab1CBSxL (ORCPT ); Wed, 2 Mar 2011 13:53:11 -0500 Content-Disposition: inline In-Reply-To: <20110302092314.GB27437@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, Mar 02, 2011 at 11:23:14AM +0200, Michael S. Tsirkin wrote: > On Wed, Mar 02, 2011 at 03:26:53PM +0800, Sheng Yang wrote: > > Change from v10: > > 1. Update according to the comments of Michael. > > 2. Use mmio_needed to exit to userspace according to Marcelo's comments. > > PCI-wise, I don't see anything to complain about. > So ack the PCI bits. > You guys decide on the rest. > > Several things I suggested previously that are not > related to the PCI point of view: > > 1. In msix_table_mmio_write, we fill in ext_data even if > we are not going to exit to userspace in the end. > It seems a trivial optimization to only do it if we exit. > 2. Instead of filling in ext_data, and then copying to vcpu, > we could fill the data in vcpu directly. > 3. MSIX is not an error. So returning -ENOTSYNC to signal > it is ugly. It would be cleaner to return negative > value on error, and positive exit code to trigger exit. > 4. Patch 4/4 adds whitespace errors that git complains about. > > With changes 2 and 3, arch/x86/kvm/x86.c would not > need to know about msix at all. > > As I said these are all suggestions unrelated to pci, > and I don't know what Avi/Marcelo think about 1 and 2. > 3 and 4 are easy to fix though. All minor IMO (i prefer ENOTSYNC as its meaningful), whitespace can be fixed while applying. Avi, can you please ACK? Sheng, we will fix any further comments. Thanks!