s390x/pci: mark zpci devices as unmigratable

[Qemu-devel] [PATCH] s390x/pci: mark zpci devices as unmigratable

Posted by Cornelia Huck 5 years, 2 months ago

We currently don't migrate any state for zpci devices, which are
coupled with standard pci devices. This means funny things happen
when we e.g. try to migrate with a virtio-pci device but the s390x-
specific zpci state is not migrated (vfio-pci is not affected, as
it is not migratable anyway.)

Until this is fixed, mark zpci devices as unmigratable.

Reported-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---

This is just a stop-gap measure to give us time to implement the
needed migration code properly.

---
 hw/s390x/s390-pci-bus.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index c96a7cba34..96c7c18f3f 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -1253,6 +1253,15 @@ static Property s390_pci_device_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
+static const VMStateDescription s390_pci_device_vmstate = {
+    .name = TYPE_S390_PCI_DEVICE,
+    /*
+     * TODO: add state handling here, so migration works at least with
+     * emulated pci devices on s390x
+     */
+    .unmigratable = 1,
+};
+
 static void s390_pci_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -1263,6 +1272,7 @@ static void s390_pci_device_class_init(ObjectClass *klass, void *data)
     dc->bus_type = TYPE_S390_PCI_BUS;
     dc->realize = s390_pci_device_realize;
     dc->props = s390_pci_device_properties;
+    dc->vmsd = &s390_pci_device_vmstate;
 }
 
 static const TypeInfo s390_pci_device_info = {
-- 
2.17.2

Re: [Qemu-devel] [PATCH] s390x/pci: mark zpci devices as unmigratable

Posted by David Hildenbrand 5 years, 2 months ago

On 01.02.19 13:45, Cornelia Huck wrote:
> We currently don't migrate any state for zpci devices, which are
> coupled with standard pci devices. This means funny things happen
> when we e.g. try to migrate with a virtio-pci device but the s390x-
> specific zpci state is not migrated (vfio-pci is not affected, as
> it is not migratable anyway.)
> 
> Until this is fixed, mark zpci devices as unmigratable.
> 
> Reported-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
> 
> This is just a stop-gap measure to give us time to implement the
> needed migration code properly.
> 
> ---
>  hw/s390x/s390-pci-bus.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index c96a7cba34..96c7c18f3f 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -1253,6 +1253,15 @@ static Property s390_pci_device_properties[] = {
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> +static const VMStateDescription s390_pci_device_vmstate = {
> +    .name = TYPE_S390_PCI_DEVICE,
> +    /*
> +     * TODO: add state handling here, so migration works at least with
> +     * emulated pci devices on s390x
> +     */
> +    .unmigratable = 1,
> +};
> +
>  static void s390_pci_device_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -1263,6 +1272,7 @@ static void s390_pci_device_class_init(ObjectClass *klass, void *data)
>      dc->bus_type = TYPE_S390_PCI_BUS;
>      dc->realize = s390_pci_device_realize;
>      dc->props = s390_pci_device_properties;
> +    dc->vmsd = &s390_pci_device_vmstate;
>  }
>  
>  static const TypeInfo s390_pci_device_info = {
> 

I guess this should be good enough (e.g. pci-bridge without a zpci
device should migrate "itself" I assume).

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH] s390x/pci: mark zpci devices as unmigratable

Posted by Cornelia Huck 5 years, 2 months ago

On Fri, 1 Feb 2019 14:29:47 +0100
David Hildenbrand <david@redhat.com> wrote:

> On 01.02.19 13:45, Cornelia Huck wrote:
> > We currently don't migrate any state for zpci devices, which are
> > coupled with standard pci devices. This means funny things happen
> > when we e.g. try to migrate with a virtio-pci device but the s390x-
> > specific zpci state is not migrated (vfio-pci is not affected, as
> > it is not migratable anyway.)
> > 
> > Until this is fixed, mark zpci devices as unmigratable.
> > 
> > Reported-by: David Hildenbrand <david@redhat.com>
> > Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> > ---
> > 
> > This is just a stop-gap measure to give us time to implement the
> > needed migration code properly.
> > 
> > ---
> >  hw/s390x/s390-pci-bus.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> > index c96a7cba34..96c7c18f3f 100644
> > --- a/hw/s390x/s390-pci-bus.c
> > +++ b/hw/s390x/s390-pci-bus.c
> > @@ -1253,6 +1253,15 @@ static Property s390_pci_device_properties[] = {
> >      DEFINE_PROP_END_OF_LIST(),
> >  };
> >  
> > +static const VMStateDescription s390_pci_device_vmstate = {
> > +    .name = TYPE_S390_PCI_DEVICE,
> > +    /*
> > +     * TODO: add state handling here, so migration works at least with
> > +     * emulated pci devices on s390x
> > +     */
> > +    .unmigratable = 1,
> > +};
> > +
> >  static void s390_pci_device_class_init(ObjectClass *klass, void *data)
> >  {
> >      DeviceClass *dc = DEVICE_CLASS(klass);
> > @@ -1263,6 +1272,7 @@ static void s390_pci_device_class_init(ObjectClass *klass, void *data)
> >      dc->bus_type = TYPE_S390_PCI_BUS;
> >      dc->realize = s390_pci_device_realize;
> >      dc->props = s390_pci_device_properties;
> > +    dc->vmsd = &s390_pci_device_vmstate;
> >  }
> >  
> >  static const TypeInfo s390_pci_device_info = {
> >   
> 
> I guess this should be good enough (e.g. pci-bridge without a zpci
> device should migrate "itself" I assume).

I don't think we need to do anything special for those.

We probably need to go through the whole zpci code and check where we
might miss some state handling. The original use case for that support
was vfio-pci, which probably explains why migration had been
neglected...

However, unmigratable zpci devices should be an effective stopper until
this has been fixed.

> 
> Reviewed-by: David Hildenbrand <david@redhat.com>
> 

Thanks!

Re: [Qemu-devel] [PATCH] s390x/pci: mark zpci devices as unmigratable

Posted by Collin Walling 5 years, 2 months ago

On 2/1/19 7:45 AM, Cornelia Huck wrote:
> We currently don't migrate any state for zpci devices, which are
> coupled with standard pci devices. This means funny things happen
> when we e.g. try to migrate with a virtio-pci device but the s390x-
> specific zpci state is not migrated (vfio-pci is not affected, as
> it is not migratable anyway.)
> 
> Until this is fixed, mark zpci devices as unmigratable.
> 
> Reported-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
> 
> This is just a stop-gap measure to give us time to implement the
> needed migration code properly.

I imagine this means we'll want a single migration handler that will
take care of PCI and zPCI in one go, that way the data stays coupled?

Just throwing thoughts out there.

> 
> ---
>   hw/s390x/s390-pci-bus.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index c96a7cba34..96c7c18f3f 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -1253,6 +1253,15 @@ static Property s390_pci_device_properties[] = {
>       DEFINE_PROP_END_OF_LIST(),
>   };
>   
> +static const VMStateDescription s390_pci_device_vmstate = {
> +    .name = TYPE_S390_PCI_DEVICE,
> +    /*
> +     * TODO: add state handling here, so migration works at least with
> +     * emulated pci devices on s390x
> +     */
> +    .unmigratable = 1,
> +};
> +
>   static void s390_pci_device_class_init(ObjectClass *klass, void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -1263,6 +1272,7 @@ static void s390_pci_device_class_init(ObjectClass *klass, void *data)
>       dc->bus_type = TYPE_S390_PCI_BUS;
>       dc->realize = s390_pci_device_realize;
>       dc->props = s390_pci_device_properties;
> +    dc->vmsd = &s390_pci_device_vmstate;
>   }
>   
>   static const TypeInfo s390_pci_device_info = {
> 

Good band-aid patch.!

Reviewed-by: Collin Walling <walling@linux.ibm.com>

Re: [Qemu-devel] [PATCH] s390x/pci: mark zpci devices as unmigratable

Posted by David Hildenbrand 5 years, 2 months ago

On 01.02.19 16:41, Collin Walling wrote:
> On 2/1/19 7:45 AM, Cornelia Huck wrote:
>> We currently don't migrate any state for zpci devices, which are
>> coupled with standard pci devices. This means funny things happen
>> when we e.g. try to migrate with a virtio-pci device but the s390x-
>> specific zpci state is not migrated (vfio-pci is not affected, as
>> it is not migratable anyway.)
>>
>> Until this is fixed, mark zpci devices as unmigratable.
>>
>> Reported-by: David Hildenbrand <david@redhat.com>
>> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
>> ---
>>
>> This is just a stop-gap measure to give us time to implement the
>> needed migration code properly.
> 
> I imagine this means we'll want a single migration handler that will
> take care of PCI and zPCI in one go, that way the data stays coupled?
> 
> Just throwing thoughts out there.

Guess this won't really be possible. Each device is to migrate "itself".
We'll then also have to see which parts of the host bridge requires
migration. Most probably not that easy. Migrating timers and such.
Taking care of resets...

As I am planning to end my journey to wondeful zpci land for now (but I
might eventually look into it again in the future), I won't be looking
into migration.

If IBM has some resources for that, very much appreciated.

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH] s390x/pci: mark zpci devices as unmigratable

Posted by Cornelia Huck 5 years, 2 months ago

On Fri, 1 Feb 2019 17:06:26 +0100
David Hildenbrand <david@redhat.com> wrote:

> On 01.02.19 16:41, Collin Walling wrote:
> > On 2/1/19 7:45 AM, Cornelia Huck wrote:  
> >> We currently don't migrate any state for zpci devices, which are
> >> coupled with standard pci devices. This means funny things happen
> >> when we e.g. try to migrate with a virtio-pci device but the s390x-
> >> specific zpci state is not migrated (vfio-pci is not affected, as
> >> it is not migratable anyway.)
> >>
> >> Until this is fixed, mark zpci devices as unmigratable.
> >>
> >> Reported-by: David Hildenbrand <david@redhat.com>
> >> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> >> ---
> >>
> >> This is just a stop-gap measure to give us time to implement the
> >> needed migration code properly.  
> > 
> > I imagine this means we'll want a single migration handler that will
> > take care of PCI and zPCI in one go, that way the data stays coupled?
> > 
> > Just throwing thoughts out there.  
> 
> Guess this won't really be possible. Each device is to migrate "itself".
> We'll then also have to see which parts of the host bridge requires
> migration. Most probably not that easy. Migrating timers and such.
> Taking care of resets...
> 
> As I am planning to end my journey to wondeful zpci land for now (but I
> might eventually look into it again in the future), I won't be looking
> into migration.
> 
> If IBM has some resources for that, very much appreciated.

And I'd be happy to queue such patches (just as I did right now for
this patch :)