From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51149) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xdes1-00050G-CZ for qemu-devel@nongnu.org; Mon, 13 Oct 2014 08:37:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xderv-0001SB-1T for qemu-devel@nongnu.org; Mon, 13 Oct 2014 08:37:01 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50300) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xderu-0001Ro-QE for qemu-devel@nongnu.org; Mon, 13 Oct 2014 08:36:54 -0400 Date: Mon, 13 Oct 2014 15:40:23 +0300 From: "Michael S. Tsirkin" Message-ID: <20141013124023.GA3410@redhat.com> References: <20140917192109.5ee87811@bahia.local> <20140917173925.GA19710@redhat.com> <20141006165135.5e158064@bahia.local> <20141006162621.GA2633@redhat.com> <20141006184617.37a2d886@bahia.local> <20141006172504.GA7084@redhat.com> <20141013104941.78c8b13e@bahia.local> <20141013090107.GA12539@redhat.com> <20141013124202.39a9a4f6@bahia.local> <543BC5A0.10709@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <543BC5A0.10709@suse.de> Subject: Re: [Qemu-devel] [PATCH] virtio-pci: fix migration for pci bus master List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Nikunj A Dadhania , Alexey Kardashevskiy , Jason Wang , qemu-devel@nongnu.org, Anthony Liguori On Mon, Oct 13, 2014 at 02:29:20PM +0200, Alexander Graf wrote: > > > On 13.10.14 12:42, Greg Kurz wrote: > > On Mon, 13 Oct 2014 12:01:07 +0300 > > "Michael S. Tsirkin" wrote: > > > >> On Mon, Oct 13, 2014 at 10:49:41AM +0200, Greg Kurz wrote: > >>> On Mon, 6 Oct 2014 20:25:04 +0300 > >>> "Michael S. Tsirkin" wrote: > >>>> [...] > >>>> > >>>> BTW I reverted that patch, and to fix migration, I'm thinking > >>>> about applying the following patch on top of master. > >>>> > >>> > >>> Michael, > >>> > >>> I could force the migration issue with a rhel65 guest thanks to the > >>> following patch, applied to hw/virtio/virtio-pci.c in QEMU v2.1. > >>> > >>> @@ -271,6 +272,16 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val) > >>> VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); > >>> hwaddr pa; > >>> > >>> + if ((vdev->status == (VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER)) > >>> + && (!(proxy->pci_dev.config[PCI_COMMAND] & PCI_COMMAND_MASTER)) > >>> + && getenv("MIG_BUG")) > >>> + { > >>> + fprintf(stderr, "\n\n\n\tMIGRATE !\n\n\n"); > >>> + qmp_migrate(getenv("MIG_BUG"), false, false, false, false, false, > >>> + false, NULL); > >>> + unsetenv("MIG_BUG"); > >>> + } > >>> + > >>> switch (addr) { > >>> case VIRTIO_PCI_GUEST_FEATURES: > >>> /* Guest does not negotiate properly? We have to assume nothing. */ > >>> > >>> > >>> Indeed, the destination QEMU master hangs because bus master isn't set. > >>> > >>>> Would appreciate testing cross-version migration (2.1 to master) > >>>> with this patch applied. > >>>> > >>> > >>> I had first to to enable the property for pseries of course. I could then > >>> migrate QEMU v2.1 to QEMU master and back to QEMU v2.1, in the window > >>> where we have DRIVER enabled and MASTER disabled, without experiencing > >>> the hang. > >>> > >>> Your fix works as expected (just a remark below). > >>> > >>>> > >>>> > >>>> diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h > >>>> index 1cea157..8873b6d 100644 > >>>> --- a/hw/virtio/virtio-pci.h > >>>> +++ b/hw/virtio/virtio-pci.h > >>>> @@ -53,6 +53,11 @@ typedef struct VirtioBusClass VirtioPCIBusClass; > >>>> #define VIRTIO_PCI_BUS_CLASS(klass) \ > >>>> OBJECT_CLASS_CHECK(VirtioPCIBusClass, klass, TYPE_VIRTIO_PCI_BUS) > >>>> > >>>> +/* Need to activate work-arounds for buggy guests at vmstate load. */ > >>>> +#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT 0 > >>>> +#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION \ > >>>> + (1 << VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT) > >>>> + > >>>> /* Performance improves when virtqueue kick processing is decoupled from the > >>>> * vcpu thread using ioeventfd for some devices. */ > >>>> #define VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT 1 > >>>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > >>>> index bae023a..e07b6c4 100644 > >>>> --- a/include/hw/i386/pc.h > >>>> +++ b/include/hw/i386/pc.h > >>>> @@ -312,6 +312,11 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); > >>>> .driver = "intel-hda",\ > >>>> .property = "old_msi_addr",\ > >>>> .value = "on",\ > >>>> + },\ > >>>> + {\ > >>>> + .driver = "virtio-pci",\ > >>>> + .property = "virtio-pci-bus-master-bug-migration",\ > >>>> + .value = "on",\ > >>>> } > >>>> > >>>> #define PC_COMPAT_2_0 \ > >>> > >>> FWIW, the issue does not occur with intel targets, at least not > >>> in my test case (booting rhel6 on a virtio-blk disk). I see bus > >>> master is set early (bios ?) and never unset... > >>> > >>> If you decide to apply for intel anyway, shouldn't the enablement be > >>> in a separate patch ? > >>> > >>> Will you resend or would you like me to do it, along with the pseries > >>> enablement part ? In this case, I would need your SoB for the present > >>> patch. > >>> > >>> Thanks. > >>> > >>> -- > >>> Greg > >> > >> AFAIK pseries doesn't support cross-version migration, does it? > > We're trying to make sure we don't break cross-version migration for the > pseries machines. If nothing else, at least as a learning exercise. > > > Alex I see. In that case, I think we need a common header, where we can add device compatibility defines. With PC and PSERIES would then pull it in. Let's not duplicate code. I'll try to come up with a patch. -- MST