Hi Zhenzhong,
On 1/15/24 11:13, Zhenzhong Duan wrote:
> IOMMUFDDevice represents a device in iommufd and can be used as
> a communication interface between devices (i.e., VFIO, VDPA) and
> vIOMMU.
>
> Currently it includes iommufd handler and device id information
iommufd handle
> which could be used by vIOMMU to get hw IOMMU information.
>
> In future nested translation support, vIOMMU is going to have
> more iommufd related operations like allocate hwpt for a device,
> attach/detach hwpt, etc. So IOMMUFDDevice will be further expanded.
>
> IOMMUFDDevice is willingly not a QOM object because we don't want
> it to be visible from the user interface.
>
> Introduce a helper iommufd_device_init to initialize IOMMUFDDevice.
+ iommufd_device_get_info helper
>
> Originally-by: Yi Liu <yi.l....@intel.com>
> Signed-off-by: Yi Sun <yi.y....@linux.intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
> ---
> MAINTAINERS | 4 +--
> include/sysemu/iommufd_device.h | 31 ++++++++++++++++++++
> backends/iommufd_device.c | 50 +++++++++++++++++++++++++++++++++
> backends/meson.build | 2 +-
> 4 files changed, 84 insertions(+), 3 deletions(-)
> create mode 100644 include/sysemu/iommufd_device.h
> create mode 100644 backends/iommufd_device.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 00ec1f7eca..606dfeb2b1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2171,8 +2171,8 @@ M: Yi Liu <yi.l....@intel.com>
> M: Eric Auger <eric.au...@redhat.com>
> M: Zhenzhong Duan <zhenzhong.d...@intel.com>
> S: Supported
> -F: backends/iommufd.c
> -F: include/sysemu/iommufd.h
> +F: backends/iommufd*.c
> +F: include/sysemu/iommufd*.h
> F: include/qemu/chardev_open.h
> F: util/chardev_open.c
> F: docs/devel/vfio-iommufd.rst
> diff --git a/include/sysemu/iommufd_device.h b/include/sysemu/iommufd_device.h
> new file mode 100644
> index 0000000000..795630324b
> --- /dev/null
> +++ b/include/sysemu/iommufd_device.h
> @@ -0,0 +1,31 @@
> +/*
> + * IOMMUFD Device
> + *
> + * Copyright (C) 2024 Intel Corporation.
> + *
> + * Authors: Yi Liu <yi.l....@intel.com>
> + * Zhenzhong Duan <zhenzhong.d...@intel.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef SYSEMU_IOMMUFD_DEVICE_H
> +#define SYSEMU_IOMMUFD_DEVICE_H
> +
> +#include <linux/iommufd.h>
> +#include "sysemu/iommufd.h"
> +
> +typedef struct IOMMUFDDevice IOMMUFDDevice;
> +
> +/* This is an abstraction of host IOMMUFD device */
> +struct IOMMUFDDevice {
> + IOMMUFDBackend *iommufd;
> + uint32_t dev_id;
> +};
> +
> +int iommufd_device_get_info(IOMMUFDDevice *idev,
> + enum iommu_hw_info_type *type,
> + uint32_t len, void *data);
> +void iommufd_device_init(void *_idev, size_t instance_size,
> + IOMMUFDBackend *iommufd, uint32_t dev_id);
> +#endif
> diff --git a/backends/iommufd_device.c b/backends/iommufd_device.c
> new file mode 100644
> index 0000000000..f6e7ca1dbf
> --- /dev/null
> +++ b/backends/iommufd_device.c
> @@ -0,0 +1,50 @@
> +/*
> + * QEMU abstract of Host IOMMU
it is the abstraction of the IOMMU or of any assigned device?
> + *
> + * Copyright (C) 2024 Intel Corporation.
> + *
> + * Authors: Yi Liu <yi.l....@intel.com>
> + * Zhenzhong Duan <zhenzhong.d...@intel.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include <sys/ioctl.h>
> +#include "qemu/osdep.h"
> +#include "qemu/error-report.h"
> +#include "sysemu/iommufd_device.h"
> +
> +int (IOMMUFDDevice *idev,
> + enum iommu_hw_info_type *type,
> + uint32_t len, void *data)
> +{
> + struct iommu_hw_info info = {
> + .size = sizeof(info),
> + .flags = 0,
> + .dev_id = idev->dev_id,
> + .data_len = len,
> + .__reserved = 0,
> + .data_uptr = (uintptr_t)data,
> + };
> + int ret;
> +
> + ret = ioctl(idev->iommufd->fd, IOMMU_GET_HW_INFO, &info);
> + if (ret) {
> + error_report("Failed to get info %m");
you may prefer using errp instead of hard traces.
> + } else {
> + *type = info.out_data_type;
> + }
> +
> + return ret;
> +}
> +
> +void iommufd_device_init(void *_idev, size_t instance_size,
nit: why the "_"
> + IOMMUFDBackend *iommufd, uint32_t dev_id)
> +{
> + IOMMUFDDevice *idev = (IOMMUFDDevice *)_idev;
> +
> + g_assert(sizeof(IOMMUFDDevice) <= instance_size);
at this stage of the reading it is not clear why you input the
instance_size. worth to be clarified/documented.
> +
> + idev->iommufd = iommufd;
> + idev->dev_id = dev_id;
> +}
> diff --git a/backends/meson.build b/backends/meson.build
> index 8b2b111497..c437cdb363 100644
> --- a/backends/meson.build
> +++ b/backends/meson.build
> @@ -24,7 +24,7 @@ if have_vhost_user
> system_ss.add(when: 'CONFIG_VIRTIO', if_true: files('vhost-user.c'))
> 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_IOMMUFD', if_true: files('iommufd.c',
> 'iommufd_device.c'))
> if have_vhost_user_crypto
> system_ss.add(when: 'CONFIG_VIRTIO_CRYPTO', if_true:
> files('cryptodev-vhost-user.c'))
> endif
Thanks
Eric
