Re: [qemu-s390x] [PATCH v8 4/6] s390x/ap: base Adjunct Processor (AP) object model

[Tony Krowiak]

On 09/13/2018 01:02 PM, Tony Krowiak wrote:
> On 09/13/2018 02:29 AM, Christian Borntraeger wrote:
> > On 09/13/2018 07:48 AM, Thomas Huth wrote:
> > > On 2018-09-12 22:08, Tony Krowiak wrote:
> > > > From: Tony Krowiak <address@hidden>
> > > >
> > > > Introduces the base object model for virtualizing AP devices.
> > > >
> > > > Signed-off-by: Tony Krowiak <address@hidden>
> > > > ---
> > > [...]
> > > > diff --git a/hw/s390x/ap-bridge.c b/hw/s390x/ap-bridge.c
> > > > new file mode 100644
> > > > index 0000000..4f10425
> > > > --- /dev/null
> > > > +++ b/hw/s390x/ap-bridge.c
> > > [...]
> > > > +void s390_init_ap(void)
> > > > +{
> > > > +    DeviceState *dev;
> > > > +
> > > > +    /* Create bridge device */
> > > > +    dev = qdev_create(NULL, TYPE_AP_BRIDGE);
> > > > +    object_property_add_child(qdev_get_machine(), TYPE_AP_BRIDGE,
> > > > +                              OBJECT(dev), NULL);
> > > > +    qdev_init_nofail(dev);
> > > > +
> > > > +    /* Create bus on bridge device */
> > > > +    qbus_create(TYPE_VFIO_AP_BUS, dev, TYPE_VFIO_AP_BUS);
> > > > + }
> > > [...]
> > > > +type_init(ap_device_register)
> > > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> > > > index f0f7fdc..3c100c2 100644
> > > > --- a/hw/s390x/s390-virtio-ccw.c
> > > > +++ b/hw/s390x/s390-virtio-ccw.c
> > > > @@ -32,6 +32,7 @@
> > > >   #include "ipl.h"
> > > >   #include "hw/s390x/s390-virtio-ccw.h"
> > > >   #include "hw/s390x/css-bridge.h"
> > > > +#include "hw/s390x/ap-bridge.h"
> > > >   #include "migration/register.h"
> > > >   #include "cpu_models.h"
> > > >   #include "hw/nmi.h"
> > > > @@ -263,6 +264,9 @@ static void ccw_init(MachineState *machine)
> > > >       /* init the SIGP facility */
> > > >       s390_init_sigp();
> > > >   +    /* create AP bridge and bus(es) */
> > > > +    s390_init_ap();
> > > > +
> > > Not sure since there is no explicit migration state involved here, 
> > > but I
> > > think you cannot simply create the ap-bridge device always
> > > unconditionally, can you? Did you check whether you can ping-pong
> > > migrate an guest that runs on an older version of QEMU to a QEMU that
> > > contains this patch and back? If it does not work, it might be 
> > > necessary
> > > to restrict this ap-bridge device for new machine types only, I think.
> > We have already the cpu features at this point in time. So simply doing
> > s390_has_feat(S390_FEAT_AP) in s390_init_ap ?
>
> This seems like a reasonable solution to me. If the S390_FEAT_AP is 
> not switched
> on for the guest, then the vfio-ap device will not be realized; 
> therefore,
> there is no need for a bridge device. I will fix the s390_init_ap() 
> function such
> that the AP bridge device is created only if the S390_FEAT_AP is 
> switched on for
> the guest. That ought to solve the migration issue David pointed out. 
> Thanks
> David.

Sorry, meant Thomas.

>