All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Sheng Yang <sheng@linux.intel.com>
Cc: Avi Kivity <avi@redhat.com>,
	Marcelo Tosatti <mtosatti@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 11:23:14 +0200	[thread overview]
Message-ID: <20110302092314.GB27437@redhat.com> (raw)
In-Reply-To: <1299050817-30792-1-git-send-email-sheng@linux.intel.com>

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.


> Sheng Yang (4):
>   KVM: Move struct kvm_io_device to kvm_host.h
>   KVM: Add kvm_io_ext_data to IO handler
>   KVM: Emulate MSI-X table in kernel
>   KVM: Add documents for MSI-X MMIO API
> 
>  Documentation/kvm/api.txt |   58 +++++++++
>  arch/x86/kvm/Makefile     |    2 +-
>  arch/x86/kvm/i8254.c      |    6 +-
>  arch/x86/kvm/i8259.c      |    3 +-
>  arch/x86/kvm/lapic.c      |    3 +-
>  arch/x86/kvm/x86.c        |   42 +++++--
>  include/linux/kvm.h       |   28 +++++
>  include/linux/kvm_host.h  |   64 ++++++++++-
>  virt/kvm/assigned-dev.c   |   41 +++++++
>  virt/kvm/coalesced_mmio.c |    3 +-
>  virt/kvm/eventfd.c        |    2 +-
>  virt/kvm/ioapic.c         |    2 +-
>  virt/kvm/iodev.h          |   31 +----
>  virt/kvm/kvm_main.c       |   40 ++++++-
>  virt/kvm/msix_mmio.c      |  291 +++++++++++++++++++++++++++++++++++++++++++++
>  virt/kvm/msix_mmio.h      |   26 ++++
>  16 files changed, 591 insertions(+), 51 deletions(-)
>  create mode 100644 virt/kvm/msix_mmio.c
>  create mode 100644 virt/kvm/msix_mmio.h

  parent reply	other threads:[~2011-03-02  9:23 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 ` Michael S. Tsirkin [this message]
2011-03-02 18:51   ` [PATCH 0/4 v12] MSI-X MMIO support for KVM Marcelo Tosatti
2011-03-02 19:56     ` Michael S. Tsirkin
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=20110302092314.GB27437@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.