From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49850) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xdekp-0001uL-1p for qemu-devel@nongnu.org; Mon, 13 Oct 2014 08:29:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xdekh-000731-HD for qemu-devel@nongnu.org; Mon, 13 Oct 2014 08:29:35 -0400 Received: from cantor2.suse.de ([195.135.220.15]:34102 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xdekh-00072W-7R for qemu-devel@nongnu.org; Mon, 13 Oct 2014 08:29:27 -0400 Message-ID: <543BC5A0.10709@suse.de> Date: Mon, 13 Oct 2014 14:29:20 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1410719415-23769-1-git-send-email-mst@redhat.com> <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> In-Reply-To: <20141013124202.39a9a4f6@bahia.local> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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: Greg Kurz , "Michael S. Tsirkin" Cc: Alexey Kardashevskiy , Jason Wang , Nikunj A Dadhania , qemu-devel@nongnu.org, Anthony Liguori 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