Hi Philippe,

>-----Original Message-----
>From: Philippe Mathieu-Daudé <phi...@linaro.org>
>Subject: Re: [PATCH v4 01/19] backends: Introduce HostIOMMUDevice
>abstract
>
>Hi Zhenzhong,
>
>On 7/5/24 11:20, Zhenzhong Duan wrote:
>> Introduce HostIOMMUDevice as an abstraction of host IOMMU device.
>>
>> Introduce .realize() to initialize HostIOMMUDevice further after
>> instance init.
>>
>> Introduce a macro CONFIG_HOST_IOMMU_DEVICE to define the usage
>> for VFIO, and VDPA in the future.
>>
>> Suggested-by: Cédric Le Goater <c...@redhat.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
>> ---
>>   MAINTAINERS                        |  2 ++
>>   include/sysemu/host_iommu_device.h | 51
>++++++++++++++++++++++++++++++
>>   backends/host_iommu_device.c       | 30 ++++++++++++++++++
>>   backends/Kconfig                   |  5 +++
>>   backends/meson.build               |  1 +
>>   5 files changed, 89 insertions(+)
>>   create mode 100644 include/sysemu/host_iommu_device.h
>>   create mode 100644 backends/host_iommu_device.c
>
>
>> diff --git a/include/sysemu/host_iommu_device.h
>b/include/sysemu/host_iommu_device.h
>> new file mode 100644
>> index 0000000000..2b58a94d62
>> --- /dev/null
>> +++ b/include/sysemu/host_iommu_device.h
>> @@ -0,0 +1,51 @@
>> +/*
>> + * Host IOMMU device abstract declaration
>> + *
>> + * Copyright (C) 2024 Intel Corporation.
>> + *
>> + * Authors: Zhenzhong Duan <zhenzhong.d...@intel.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2.  See
>> + * the COPYING file in the top-level directory.
>> + */
>> +
>> +#ifndef HOST_IOMMU_DEVICE_H
>> +#define HOST_IOMMU_DEVICE_H
>> +
>> +#include "qom/object.h"
>> +#include "qapi/error.h"
>> +
>> +#define TYPE_HOST_IOMMU_DEVICE "host-iommu-device"
>> +OBJECT_DECLARE_TYPE(HostIOMMUDevice, HostIOMMUDeviceClass,
>HOST_IOMMU_DEVICE)
>> +
>> +struct HostIOMMUDevice {
>> +    Object parent_obj;
>> +};
>> +
>> +/**
>> + * struct HostIOMMUDeviceClass - The base class for all host IOMMU
>devices.
>> + *
>> + * Different type of host devices (e.g., VFIO or VDPA device) or devices
>> + * with different backend (e.g., VFIO legacy container or IOMMUFD
>backend)
>> + * can have different sub-classes.
>> + */
>> +struct HostIOMMUDeviceClass {
>> +    ObjectClass parent_class;
>> +
>> +    /**
>> +     * @realize: initialize host IOMMU device instance further.
>> +     *
>> +     * Mandatory callback.
>> +     *
>> +     * @hiod: pointer to a host IOMMU device instance.
>> +     *
>> +     * @opaque: pointer to agent device of this host IOMMU device,
>> +     *          i.e., for VFIO, pointer to VFIODevice
>> +     *
>> +     * @errp: pass an Error out when realize fails.
>> +     *
>> +     * Returns: true on success, false on failure.
>> +     */
>> +    bool (*realize)(HostIOMMUDevice *hiod, void *opaque, Error **errp);
>> +};
>> +#endif
>> diff --git a/backends/host_iommu_device.c
>b/backends/host_iommu_device.c
>> new file mode 100644
>> index 0000000000..41f2fdce20
>> --- /dev/null
>> +++ b/backends/host_iommu_device.c
>> @@ -0,0 +1,30 @@
>> +/*
>> + * Host IOMMU device abstract
>> + *
>> + * Copyright (C) 2024 Intel Corporation.
>> + *
>> + * Authors: Zhenzhong Duan <zhenzhong.d...@intel.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2.  See
>> + * the COPYING file in the top-level directory.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "sysemu/host_iommu_device.h"
>> +
>> +OBJECT_DEFINE_ABSTRACT_TYPE(HostIOMMUDevice,
>> +                            host_iommu_device,
>> +                            HOST_IOMMU_DEVICE,
>> +                            OBJECT)
>> +
>> +static void host_iommu_device_class_init(ObjectClass *oc, void *data)
>> +{
>> +}
>> +
>> +static void host_iommu_device_init(Object *obj)
>> +{
>> +}
>> +
>> +static void host_iommu_device_finalize(Object *obj)
>> +{
>> +}
>
>All these stubs call for a QOM interface design instead
>of inheritance IMHO. See INTERFACE_CHECK in "qom/object.h".
>But maybe I misunderstood this series :)

Thanks for your suggestion. Guess you mean introducing an interface
which contains .realize() and .get_cap(), let HostIOMMUDevice or its
sub-classes implement this interface.

.realize() and .get_cap() are not common handlers and only used by host iommu
device QOM, same for such an interface. So it looks an over design. We can let
HostIOMMUDevice or its sub-classes define and implement those handlers directly.

Thanks
Zhenzhong

Reply via email to