public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Avi Kivity <avi@redhat.com>,
	Marcelo Tosatti <mtosatti@redhat.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>
Subject: Re: [PATCH v2 8/9] qemu-kvm: Refresh MSI settings after vmload
Date: Wed, 27 Apr 2011 17:30:49 +0300	[thread overview]
Message-ID: <20110427143049.GQ15788@redhat.com> (raw)
In-Reply-To: <4DB82828.7040800@siemens.com>

On Wed, Apr 27, 2011 at 04:28:56PM +0200, Jan Kiszka wrote:
> On 2011-04-27 16:16, Michael S. Tsirkin wrote:
> > On Wed, Apr 27, 2011 at 01:39:05PM +0200, Jan Kiszka wrote:
> >> On 2011-04-27 11:34, Avi Kivity wrote:
> >>> On 04/26/2011 04:19 PM, Jan Kiszka wrote:
> >>>> I've still plans to consolidate MSI-X mask notifiers and KVM hooks, but
> >>>> that can wait until we go upstream.
> >>>>
> >>>> This version still makes classic MSI usable in irqchip mode, now not
> >>>> only for PCI devices (AHCI, HDA) but also for the HPET (with msi=on).
> >>>> Moreover, it contains an additional patch to refresh the MSI IRQ routes
> >>>> after vmload.
> >>>>
> >>>
> >>> Immediately after migration:
> >>>
> >>> Program terminated with signal 11, Segmentation fault.
> >>> #0  le32_to_cpupu (vector=0, kmm=0x0, dev=<value optimized out>) at 
> >>> ../bswap.h:178
> >>> 178        return p1[0] | (p1[1] << 8) | (p1[2] << 16) | (p1[3] << 24);
> >>> Missing separate debuginfos, use: debuginfo-install 
> >>> SDL-1.2.14-10.fc14.x86_64 cyrus-sasl-lib-2.1.23-12.fc14.x86_64 
> >>> cyrus-sasl-plain-2.1.23-12.fc14.x86_64 db4-4.8.30-2.fc14.x86_64 
> >>> glibc-2.13-1.x86_64 gnutls-2.8.6-2.fc14.x86_64 
> >>> keyutils-libs-1.2-6.fc12.x86_64 krb5-libs-1.8.2-9.fc14.x86_64 
> >>> libX11-1.3.4-4.fc14.x86_64 libXau-1.0.6-1.fc14.x86_64 
> >>> libaio-0.3.109-2.fc13.x86_64 libcom_err-1.41.12-6.fc14.x86_64 
> >>> libcurl-7.21.0-6.fc14.x86_64 libgcc-4.5.1-4.fc14.x86_64 
> >>> libgcrypt-1.4.5-4.fc13.x86_64 libgpg-error-1.9-1.fc14.x86_64 
> >>> libidn-1.18-1.fc14.x86_64 libpng-1.2.44-1.fc14.x86_64 
> >>> libselinux-2.0.96-6.fc14.1.x86_64 libssh2-1.2.4-1.fc14.x86_64 
> >>> libtasn1-2.7-1.fc14.x86_64 libxcb-1.7-1.fc14.x86_64 
> >>> ncurses-libs-5.7-9.20100703.fc14.x86_64 nspr-4.8.7-1.fc14.x86_64 
> >>> nss-3.12.9-9.fc14.x86_64 nss-softokn-freebl-3.12.9-5.fc14.x86_64 
> >>> nss-util-3.12.9-1.fc14.x86_64 openldap-2.4.23-4.fc14.x86_64 
> >>> openssl-1.0.0d-1.fc14.x86_64 zlib-1.2.5-2.fc14.x86_64
> >>> (gdb) bt
> >>> #0  le32_to_cpupu (vector=0, kmm=0x0, dev=<value optimized out>) at 
> >>> ../bswap.h:178
> >>> #1  pci_get_long (vector=0, kmm=0x0, dev=<value optimized out>) at 
> >>> /build/home/tlv/akivity/qemu-kvm/hw/pci.h:326
> >>> #2  kvm_msi_message_from_vector (vector=0, kmm=0x0, dev=<value optimized 
> >>> out>) at /build/home/tlv/akivity/qemu-kvm/hw/msi.c:120
> >>> #3  0x000000000057d59c in kvm_msi_update (dev=0x23eabb0) at 
> >>> /build/home/tlv/akivity/qemu-kvm/hw/msi.c:152
> >>> #4  0x000000000041e29b in get_pci_config_device (f=0x2466380, 
> >>> pv=0x23eac28, size=256) at /build/home/tlv/akivity/qemu-kvm/hw/pci.c:346
> >>> #5  0x000000000049c36c in vmstate_load_state (f=0x2466380, 
> >>> vmsd=0x5fb880, opaque=0x23eabb0, version_id=2) at savevm.c:1374
> >>> #6  0x000000000049c323 in vmstate_load_state (f=0x2466380, 
> >>> vmsd=0x6f07c0, opaque=0x23eabb0, version_id=3) at savevm.c:1372
> >>> #7  0x000000000049cf84 in vmstate_load (f=0x2466380) at savevm.c:1450
> >>> #8  qemu_loadvm_state (f=0x2466380) at savevm.c:1817
> >>> #9  0x0000000000493e69 in process_incoming_migration (f=<value optimized 
> >>> out>) at migration.c:66
> >>> #10 0x0000000000494b97 in tcp_accept_incoming_migration (opaque=<value 
> >>> optimized out>) at migration-tcp.c:163
> >>> #11 0x00000000004a3fa7 in qemu_iohandler_poll (readfds=0x7fff56dc0430, 
> >>> writefds=0x7fff56dc03b0, xfds=<value optimized out>,
> >>>      ret=<value optimized out>) at iohandler.c:120
> >>> #12 0x000000000041944a in main_loop_wait (nonblocking=<value optimized 
> >>> out>) at /build/home/tlv/akivity/qemu-kvm/vl.c:1336
> >>> #13 0x0000000000433a97 in kvm_main_loop () at 
> >>> /build/home/tlv/akivity/qemu-kvm/qemu-kvm.c:1588
> >>> #14 0x000000000041a3a6 in main_loop (argc=<value optimized out>, 
> >>> argv=<value optimized out>, envp=<value optimized out>)
> >>>      at /build/home/tlv/akivity/qemu-kvm/vl.c:1369
> >>> #15 main (argc=<value optimized out>, argv=<value optimized out>, 
> >>> envp=<value optimized out>) at /build/home/tlv/akivity/qemu-kvm/vl.c:3257
> >>>
> >>> (gdb) fr
> >>> #3  0x000000000057d59c in kvm_msi_update (dev=0x23eabb0) at 
> >>> /build/home/tlv/akivity/qemu-kvm/hw/msi.c:152
> >>> (gdb) p dev.msi_irq_entries
> >>> $10 = (struct KVMMsiMessage *) 0x0
> >>>
> >>> dev points to the i440fx chipset device.
> >>>
> >>
> >> Yeah, better use this version of patch 8.
> >>
> >> Jan
> >>
> >> ----8<-----
> >>
> >> Establish a post-load notification for the MSI subsystem so that KVM can
> >> refresh its IRQ routing after vmload.
> >>
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >> ---
> >>
> >> v3: Fix null-pointer deref after vmload by checking for availability of
> >>     msi routing entries.
> >>
> >>  hw/msi.c |   13 +++++++++++++
> >>  hw/msi.h |    1 +
> >>  hw/pci.c |    2 ++
> >>  3 files changed, 16 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/hw/msi.c b/hw/msi.c
> >> index 18f683b..725b2b7 100644
> >> --- a/hw/msi.c
> >> +++ b/hw/msi.c
> >> @@ -453,3 +453,16 @@ unsigned int msi_nr_vectors_allocated(const PCIDevice *dev)
> >>      uint16_t flags = pci_get_word(dev->config + msi_flags_off(dev));
> >>      return msi_nr_vectors(flags);
> >>  }
> >> +
> >> +void msi_post_load(PCIDevice *dev)
> >> +{
> >> +    uint16_t flags = pci_get_word(dev->config + msi_flags_off(dev));
> >> +
> >> +    if (kvm_enabled() && dev->msi_irq_entries) {
> >> +        kvm_msi_free(dev);
> >> +
> >> +        if (flags & PCI_MSI_FLAGS_ENABLE) {
> >> +            kvm_msi_update(dev);
> >> +        }
> >> +    }
> >> +}
> >> diff --git a/hw/msi.h b/hw/msi.h
> >> index 5766018..6ff0607 100644
> >> --- a/hw/msi.h
> >> +++ b/hw/msi.h
> >> @@ -32,6 +32,7 @@ void msi_reset(PCIDevice *dev);
> >>  void msi_notify(PCIDevice *dev, unsigned int vector);
> >>  void msi_write_config(PCIDevice *dev, uint32_t addr, uint32_t val, int len);
> >>  unsigned int msi_nr_vectors_allocated(const PCIDevice *dev);
> >> +void msi_post_load(PCIDevice *dev);
> >>  
> >>  static inline bool msi_present(const PCIDevice *dev)
> >>  {
> >> diff --git a/hw/pci.c b/hw/pci.c
> >> index 82e0300..07ec4f9 100644
> >> --- a/hw/pci.c
> >> +++ b/hw/pci.c
> >> @@ -34,6 +34,7 @@
> >>  #include "device-assignment.h"
> >>  #include "qemu-objects.h"
> >>  #include "range.h"
> >> +#include "msi.h"
> >>  
> >>  //#define DEBUG_PCI
> >>  #ifdef DEBUG_PCI
> >> @@ -342,6 +343,7 @@ static int get_pci_config_device(QEMUFile *f, void *pv, size_t size)
> >>      memcpy(s->config, config, size);
> >>  
> >>      pci_update_mappings(s);
> >> +    msi_post_load(s);
> > 
> > Pls don't do this: I'm trying to keep just the core in
> > pci.c and all capabilities in separate files.
> > msix has msix_load, msi will just need one too,
> > and let all devices call that.
> > 
> 
> Preferred alternatives are...? Registering a vmstate for msi?
> 
> Jan

Add msi_load and call that from devices that need it.
Like msix_load does now.

> -- 
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux

  reply	other threads:[~2011-04-27 14:31 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-26 13:19 [PATCH v2 0/9] qemu-kvm: Clean up and enhance MSI irqchip support Jan Kiszka
2011-04-26 13:19 ` [PATCH v2 1/9] qemu-kvm: Drop unneeded kvm_irq_routing_entry declaration Jan Kiszka
2011-04-26 13:19 ` [PATCH v2 2/9] qemu-kvm: Rename kvm_msix_message to KVMMsiMessage Jan Kiszka
2011-04-26 13:19 ` [PATCH v2 3/9] qemu-kvm: Refactor MSI core API of KVM Jan Kiszka
2011-04-26 13:19 ` [PATCH v2 4/9] qemu-kvm: Fix and clean up msix vector use/unuse hooks Jan Kiszka
2011-04-26 13:19 ` [PATCH v2 5/9] qemu-kvm: Move gsi bits from kvm_msix_vector_add to kvm_msi_add_message Jan Kiszka
2011-04-26 13:19 ` [PATCH v2 6/9] qemu-kvm: Move entry comparison into kvm_msi_update_message Jan Kiszka
2011-04-26 13:19 ` [PATCH v2 7/9] qemu-kvm: Add in-kernel irqchip support for MSI Jan Kiszka
2011-04-26 13:19 ` [PATCH v2 8/9] qemu-kvm: Refresh MSI settings after vmload Jan Kiszka
2011-04-26 13:19 ` [PATCH v2 9/9] qemu-kvm: hpet: Add MSI support for in-kernel irqchip mode Jan Kiszka
2011-04-26 13:30   ` Michael Tokarev
2011-04-26 13:55     ` Jan Kiszka
2011-04-26 13:56       ` Avi Kivity
2011-04-26 13:58         ` Jan Kiszka
2011-04-26 14:01   ` [PATCH v3 " Jan Kiszka
2011-04-26 16:06     ` Christoph Hellwig
2011-04-26 17:06       ` Jan Kiszka
2011-04-26 17:09         ` Christoph Hellwig
2011-04-27  7:27 ` [PATCH v2 0/9] qemu-kvm: Clean up and enhance MSI irqchip support Avi Kivity
2011-04-27  9:00   ` Jan Kiszka
2011-04-27  9:04     ` Avi Kivity
2011-04-27  9:06       ` Jan Kiszka
2011-04-27  9:14         ` Avi Kivity
2011-04-27 11:21           ` Jan Kiszka
2011-04-27 12:12             ` Avi Kivity
2011-04-27 13:31               ` Jan Kiszka
2011-04-27 13:39                 ` Avi Kivity
2011-04-27 13:54                   ` Jan Kiszka
2011-04-27 14:01                     ` Avi Kivity
2011-04-27 14:11                       ` Michael S. Tsirkin
2011-04-27 14:02                     ` Michael S. Tsirkin
2011-04-27 14:10                       ` Jan Kiszka
2011-04-27 14:14                         ` Michael S. Tsirkin
2011-04-27 14:21                           ` Jan Kiszka
2011-04-27 13:49                 ` Michael S. Tsirkin
2011-04-27  9:34 ` Avi Kivity
2011-04-27 11:39   ` [PATCH v2 8/9] qemu-kvm: Refresh MSI settings after vmload Jan Kiszka
2011-04-27 12:19     ` Avi Kivity
2011-04-27 14:16     ` Michael S. Tsirkin
2011-04-27 14:28       ` Jan Kiszka
2011-04-27 14:30         ` Michael S. Tsirkin [this message]
2011-04-27 14:39           ` Jan Kiszka
2011-04-27 15:09             ` Michael S. Tsirkin
2011-04-27 15:21               ` Jan Kiszka
2011-04-27 16:02                 ` Michael S. Tsirkin
2011-04-27 16:20                   ` Jan Kiszka
2011-04-27 16:26                     ` Michael S. Tsirkin

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=20110427143049.GQ15788@redhat.com \
    --to=mst@redhat.com \
    --cc=avi@redhat.com \
    --cc=jan.kiszka@siemens.com \
    --cc=kvm@vger.kernel.org \
    --cc=mtosatti@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox