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

[Tony Krowiak]

On 11/29/18 6:45 AM, 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?

It is not possible to have interpretation and interception. I suppose
one could mix emulated and virtual AP devices, but that seems highly
unlikely; in fact, I think it is highly unlikely that emulation is ever
implemented.

> > +    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?

I received a comment from Thomas Huth in Message ID
<address@hidden>
that DO_UPCAST should be avoided in new code. This macro
should probably be restored and an AP_DEVICE() macro added.

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