Re: [PATCH v2 01/10] backends: Introduce abstract HostIOMMUDevice

["Cédric Le Goater" <clg@redhat.com>]

On 4/8/24 10:12, Zhenzhong Duan wrote:
> Introduce HostIOMMUDevice as an abstraction of host IOMMU device.
> 
> get_host_iommu_info() is used to get host IOMMU info, different
> backends can have different implementations and result format.
> 
> Introduce a macro CONFIG_HOST_IOMMU_DEVICE to define the usage
> for VFIO, and VDPA in the future.
> 
> Suggested-by: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

LGTM,

> ---
>   MAINTAINERS                        |  2 ++
>   include/sysemu/host_iommu_device.h | 19 +++++++++++++++++++
>   backends/host_iommu_device.c       | 19 +++++++++++++++++++
>   backends/Kconfig                   |  5 +++++
>   backends/meson.build               |  1 +
>   5 files changed, 46 insertions(+)
>   create mode 100644 include/sysemu/host_iommu_device.h
>   create mode 100644 backends/host_iommu_device.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e71183eef9..22f71cbe02 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2202,6 +2202,8 @@ M: Zhenzhong Duan <zhenzhong.duan@intel.com>
>   S: Supported
>   F: backends/iommufd.c
>   F: include/sysemu/iommufd.h
> +F: backends/host_iommu_device.c
> +F: include/sysemu/host_iommu_device.h
>   F: include/qemu/chardev_open.h
>   F: util/chardev_open.c
>   F: docs/devel/vfio-iommufd.rst
> diff --git a/include/sysemu/host_iommu_device.h b/include/sysemu/host_iommu_device.h
> new file mode 100644
> index 0000000000..22ccbe3a5d
> --- /dev/null
> +++ b/include/sysemu/host_iommu_device.h
> @@ -0,0 +1,19 @@
> +#ifndef HOST_IOMMU_DEVICE_H
> +#define HOST_IOMMU_DEVICE_H
> +
> +#include "qom/object.h"
> +
> +#define TYPE_HOST_IOMMU_DEVICE "host-iommu-device"
> +OBJECT_DECLARE_TYPE(HostIOMMUDevice, HostIOMMUDeviceClass, HOST_IOMMU_DEVICE)
> +
> +struct HostIOMMUDevice {
> +    Object parent;
> +};
> +
> +struct HostIOMMUDeviceClass {
> +    ObjectClass parent_class;

Could you please document the struct and its handlers ? This is more for
the future reader to understand the VFIO concepts than for the generated
docs. Anyhow, it could be useful for the docs also. Overall, the QEMU VFIO
susbsytem suffers from a lack of documentation and we should try to improve
that in the next cycle.

Thanks,

C.



> +    int (*get_host_iommu_info)(HostIOMMUDevice *hiod, void *data, uint32_t len,
> +                               Error **errp);
> +};
> +#endif
> diff --git a/backends/host_iommu_device.c b/backends/host_iommu_device.c
> new file mode 100644
> index 0000000000..6cb6007d8c
> --- /dev/null
> +++ b/backends/host_iommu_device.c
> @@ -0,0 +1,19 @@
> +#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)
> +{
> +}
> diff --git a/backends/Kconfig b/backends/Kconfig
> index 2cb23f62fa..34ab29e994 100644
> --- a/backends/Kconfig
> +++ b/backends/Kconfig
> @@ -3,3 +3,8 @@ source tpm/Kconfig
>   config IOMMUFD
>       bool
>       depends on VFIO
> +
> +config HOST_IOMMU_DEVICE
> +    bool
> +    default y
> +    depends on VFIO
> diff --git a/backends/meson.build b/backends/meson.build
> index 8b2b111497..2e975d641e 100644
> --- a/backends/meson.build
> +++ b/backends/meson.build
> @@ -25,6 +25,7 @@ if have_vhost_user
>   endif
>   system_ss.add(when: 'CONFIG_VIRTIO_CRYPTO', if_true: files('cryptodev-vhost.c'))
>   system_ss.add(when: 'CONFIG_IOMMUFD', if_true: files('iommufd.c'))
> +system_ss.add(when: 'CONFIG_HOST_IOMMU_DEVICE', if_true: files('host_iommu_device.c'))
>   if have_vhost_user_crypto
>     system_ss.add(when: 'CONFIG_VIRTIO_CRYPTO', if_true: files('cryptodev-vhost-user.c'))
>   endif