Re: [qemu-s390x] [PATCH v2 2/6] s390x/vfio: ap: Use the APdevice as a child of the APBus

[Pierre Morel]

On 29/11/2018 12:45, Cornelia Huck wrote:
> On Thu, 22 Nov 2018 17:35:51 +0100
> Pierre Morel <address@hidden> wrote:
>
> > Two good reasons to use the base device as a child of the
> > AP BUS:
> > - We can easily find the device without traversing the qtree.
> > - In case we have different APdevice instantiation, VFIO with
> >    interception or emulation, we will need the APDevice as
> >    a parent device.
> >
> > Signed-off-by: Pierre Morel <address@hidden>
> > ---
> >   hw/s390x/ap-device.c         | 22 ++++++++++++++++++++++
> >   hw/vfio/ap.c                 | 16 ++++++----------
> >   include/hw/s390x/ap-device.h |  2 ++
> >   3 files changed, 30 insertions(+), 10 deletions(-)
> >
> > diff --git a/hw/s390x/ap-device.c b/hw/s390x/ap-device.c
> > index f5ac8db..554d5aa 100644
> > --- a/hw/s390x/ap-device.c
> > +++ b/hw/s390x/ap-device.c
> > @@ -11,13 +11,35 @@
> >   #include "qemu/module.h"
> >   #include "qapi/error.h"
> >   #include "hw/qdev.h"
> > +#include "hw/s390x/ap-bridge.h"
> >   #include "hw/s390x/ap-device.h"
> >   
> > +APDevice *s390_get_ap(void)
> > +{
> > +    static DeviceState *apb;
> > +    BusState *bus;
> > +    BusChild *child;
> > +    static APDevice *ap;
> > +
> > +    if (ap) {
> > +        return ap;
> > +    }
> > +
> > +    apb = s390_get_ap_bridge();
> > +    /* We have only a single child on the BUS */
>
> So, there'll never a mixed environment? Or would that have a 'hybrid'
> ap device?

Not for now, we only support VFIOAPDevice and we can only have one 
single VFIO device per guest.

> > +    bus = qdev_get_child_bus(apb, TYPE_AP_BUS);
> > +    child = QTAILQ_FIRST(&bus->children);
> > +    assert(child != NULL);
> > +    ap = DO_UPCAST(APDevice, parent_obj, child->child);
> > +    return ap;
> > +}
> > +
> >   static void ap_class_init(ObjectClass *klass, void *data)
> >   {
> >       DeviceClass *dc = DEVICE_CLASS(klass);
> >   
> >       dc->desc = "AP device class";
> > +    dc->bus_type = TYPE_AP_BUS;
> >       dc->hotpluggable = false;
> >   }
> >   
> > diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
> > index 65de952..94e5a1a 100644
> > --- a/hw/vfio/ap.c
> > +++ b/hw/vfio/ap.c
> > @@ -35,9 +35,6 @@ typedef struct VFIOAPDevice {
> >       VFIODevice vdev;
> >   } VFIOAPDevice;
> >   
> > -#define VFIO_AP_DEVICE(obj) \
> > -        OBJECT_CHECK(VFIOAPDevice, (obj), VFIO_AP_DEVICE_TYPE)
>
> Hm?

We do not need this anymore.
We used to plug directly the VFIOAPDevice on the BUS but the APDevice is 
now plugged on the APBUS.

The VFIOAPDevice is plugged on the APBus through its APDevice.

This will allow to have different APDevice childs using APDevice features.
Usable for interception or emulation.


> > -
> >   static void vfio_ap_compute_needs_reset(VFIODevice *vdev)
> >   {
> >       vdev->needs_reset = false;


--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany