Re: [Qemu-devel] [PATCH] virtio-pci: fix migration for pci bus master

[Alexander Graf <agraf@suse.de>]

On 13.10.14 12:42, Greg Kurz wrote:
> On Mon, 13 Oct 2014 12:01:07 +0300
> "Michael S. Tsirkin" <mst@redhat.com> 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" <mst@redhat.com> 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