All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Marcelo Tosatti <mtosatti@redhat.com>
Cc: Sheng Yang <sheng@linux.intel.com>, Avi Kivity <avi@redhat.com>,
	Alex Williamson <alex.williamson@redhat.com>,
	kvm@vger.kernel.org
Subject: Re: [PATCH 0/4 v12] MSI-X MMIO support for KVM
Date: Wed, 2 Mar 2011 21:56:32 +0200	[thread overview]
Message-ID: <20110302195632.GC4928@redhat.com> (raw)
In-Reply-To: <20110302185120.GB21289@amt.cnet>

On Wed, Mar 02, 2011 at 03:51:20PM -0300, Marcelo Tosatti wrote:
> 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

Yes.

> (i prefer ENOTSYNC as its meaningful),

When this is merged I'll post a patch on top and
we can discuss.

> whitespace
> can be fixed while applying.
> 
> Avi, can you please ACK?
> 
> Sheng, we will fix any further comments. Thanks!

  reply	other threads:[~2011-03-02 19:56 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-02  7:26 [PATCH 0/4 v12] MSI-X MMIO support for KVM Sheng Yang
2011-03-02  7:26 ` [PATCH 1/4] KVM: Move struct kvm_io_device to kvm_host.h Sheng Yang
2011-03-02  7:26 ` [PATCH 2/4] KVM: Add kvm_io_ext_data to IO handler Sheng Yang
2011-03-02  7:26 ` [PATCH 3/4] KVM: Emulate MSI-X table in kernel Sheng Yang
2011-03-02  7:26 ` [PATCH 4/4] KVM: Add documents for MSI-X MMIO API Sheng Yang
2011-03-02  9:23 ` [PATCH 0/4 v12] MSI-X MMIO support for KVM Michael S. Tsirkin
2011-03-02 18:51   ` Marcelo Tosatti
2011-03-02 19:56     ` Michael S. Tsirkin [this message]
2011-03-03 13:31     ` Sheng Yang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110302195632.GC4928@redhat.com \
    --to=mst@redhat.com \
    --cc=alex.williamson@redhat.com \
    --cc=avi@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=mtosatti@redhat.com \
    --cc=sheng@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.