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

[Pierre Morel <pmorel@linux.ibm.com>]

On 29/11/2018 21:42, Tony Krowiak wrote:
> On 11/22/18 11:35 AM, Pierre Morel 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 <pmorel@linux.ibm.com>
>> ---
>>   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)
> 
> How about ap_get_device(void)?

Yes, keep same conventions.

> 
>> +{
>> +    static DeviceState *apb;
> 
> Why static if you call s390_get_ap_bridge()
> to retrieve it without first checking for NULL?

static are initialized as NULL.

> 
>> +    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 */
>> +    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);
> 
> I received a comment from Thomas Huth in Message ID
> <2291104a-4cbf-e4fd-3496-fa0910beb96a@redhat.com>
> that DO_UPCAST should be avoided in new code. You should
> create an AP_DEVICE macro for this in ap-device.h
> 

Thanks I will do.

Regards,
Pierre


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