[v1] Check and sync host IOMMU cap/ecap with vIOMMU

[PATCH rfcv1 1/6] backends/iommufd_device: introduce IOMMUFDDevice

Posted by Zhenzhong Duan 10 months, 2 weeks ago

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
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.

Originally-by: Yi Liu <yi.l.liu@intel.com>
Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@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.liu@intel.com>
 M: Eric Auger <eric.auger@redhat.com>
 M: Zhenzhong Duan <zhenzhong.duan@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.liu@intel.com>
+ *          Zhenzhong Duan <zhenzhong.duan@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
+ *
+ * Copyright (C) 2024 Intel Corporation.
+ *
+ * Authors: Yi Liu <yi.l.liu@intel.com>
+ *          Zhenzhong Duan <zhenzhong.duan@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 iommufd_device_get_info(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");
+    } else {
+        *type = info.out_data_type;
+    }
+
+    return ret;
+}
+
+void iommufd_device_init(void *_idev, size_t instance_size,
+                         IOMMUFDBackend *iommufd, uint32_t dev_id)
+{
+    IOMMUFDDevice *idev = (IOMMUFDDevice *)_idev;
+
+    g_assert(sizeof(IOMMUFDDevice) <= instance_size);
+
+    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
-- 
2.34.1

Re: [PATCH rfcv1 1/6] backends/iommufd_device: introduce IOMMUFDDevice

Posted by Eric Auger 10 months, 1 week ago


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
> 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.
>
> Originally-by: Yi Liu <yi.l.liu@intel.com>
> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>  MAINTAINERS                     |  4 +--
>  include/sysemu/iommufd_device.h | 31 ++++++++++++++++++++
>  backends/iommufd_device.c       | 50 +++++++++++++++++++++++++++++++++
Maybe it is still time to move the iommufd files in a sepate dir, under
hw at the same level as vfio.

Thoughts?

Eric
>  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.liu@intel.com>
>  M: Eric Auger <eric.auger@redhat.com>
>  M: Zhenzhong Duan <zhenzhong.duan@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.liu@intel.com>
> + *          Zhenzhong Duan <zhenzhong.duan@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
> + *
> + * Copyright (C) 2024 Intel Corporation.
> + *
> + * Authors: Yi Liu <yi.l.liu@intel.com>
> + *          Zhenzhong Duan <zhenzhong.duan@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 iommufd_device_get_info(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");
> +    } else {
> +        *type = info.out_data_type;
> +    }
> +
> +    return ret;
> +}
> +
> +void iommufd_device_init(void *_idev, size_t instance_size,
> +                         IOMMUFDBackend *iommufd, uint32_t dev_id)
> +{
> +    IOMMUFDDevice *idev = (IOMMUFDDevice *)_idev;
> +
> +    g_assert(sizeof(IOMMUFDDevice) <= instance_size);
> +
> +    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

RE: [PATCH rfcv1 1/6] backends/iommufd_device: introduce IOMMUFDDevice

Posted by Duan, Zhenzhong 10 months, 1 week ago


>-----Original Message-----
>From: Eric Auger <eric.auger@redhat.com>
>Subject: Re: [PATCH rfcv1 1/6] backends/iommufd_device: introduce
>IOMMUFDDevice
>
>
>
>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
>> 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.
>>
>> Originally-by: Yi Liu <yi.l.liu@intel.com>
>> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>  MAINTAINERS                     |  4 +--
>>  include/sysemu/iommufd_device.h | 31 ++++++++++++++++++++
>>  backends/iommufd_device.c       | 50
>+++++++++++++++++++++++++++++++++
>Maybe it is still time to move the iommufd files in a sepate dir, under
>hw at the same level as vfio.
>
>Thoughts?

Any reason for the movement? Hw dir contains entries to emulate different
Devices. Iommufd is not a real device. It's more a backend.

Thanks
Zhenzhong

Re: [PATCH rfcv1 1/6] backends/iommufd_device: introduce IOMMUFDDevice

Posted by Eric Auger 10 months, 1 week ago


On 1/19/24 08:31, Duan, Zhenzhong wrote:
>
>> -----Original Message-----
>> From: Eric Auger <eric.auger@redhat.com>
>> Subject: Re: [PATCH rfcv1 1/6] backends/iommufd_device: introduce
>> IOMMUFDDevice
>>
>>
>>
>> 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
>>> 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.
>>>
>>> Originally-by: Yi Liu <yi.l.liu@intel.com>
>>> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>> ---
>>>  MAINTAINERS                     |  4 +--
>>>  include/sysemu/iommufd_device.h | 31 ++++++++++++++++++++
>>>  backends/iommufd_device.c       | 50
>> +++++++++++++++++++++++++++++++++
>> Maybe it is still time to move the iommufd files in a sepate dir, under
>> hw at the same level as vfio.
>>
>> Thoughts?
> Any reason for the movement? Hw dir contains entries to emulate different
> Devices. Iommufd is not a real device. It's more a backend.
Well I was thinking it would become bigger and bigger and since you
created a new .c file with new abstraction (devices) the backend dir
flat layout may not be well adapted. But as suggested by Cédric we may
use the existing files.

Thanks

Eric
>
> Thanks
> Zhenzhong

Re: [PATCH rfcv1 1/6] backends/iommufd_device: introduce IOMMUFDDevice

Posted by Cédric Le Goater 10 months, 1 week ago

On 1/19/24 08:31, Duan, Zhenzhong wrote:
> 
> 
>> -----Original Message-----
>> From: Eric Auger <eric.auger@redhat.com>
>> Subject: Re: [PATCH rfcv1 1/6] backends/iommufd_device: introduce
>> IOMMUFDDevice
>>
>>
>>
>> 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
>>> 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.
>>>
>>> Originally-by: Yi Liu <yi.l.liu@intel.com>
>>> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>> ---
>>>   MAINTAINERS                     |  4 +--
>>>   include/sysemu/iommufd_device.h | 31 ++++++++++++++++++++
>>>   backends/iommufd_device.c       | 50
>> +++++++++++++++++++++++++++++++++
>> Maybe it is still time to move the iommufd files in a sepate dir, under
>> hw at the same level as vfio.
>>
>> Thoughts?
> 
> Any reason for the movement? Hw dir contains entries to emulate different
> Devices. Iommufd is not a real device. It's more a backend.

I would include the new services in the existing iommufd .[ch] files.
No need for a new file since the changes are all related to the IOMMUFD
device usage.

Thanks,

C.

RE: [PATCH rfcv1 1/6] backends/iommufd_device: introduce IOMMUFDDevice

Posted by Duan, Zhenzhong 10 months, 1 week ago


>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Subject: Re: [PATCH rfcv1 1/6] backends/iommufd_device: introduce
>IOMMUFDDevice
>
>On 1/19/24 08:31, Duan, Zhenzhong wrote:
>>
>>
>>> -----Original Message-----
>>> From: Eric Auger <eric.auger@redhat.com>
>>> Subject: Re: [PATCH rfcv1 1/6] backends/iommufd_device: introduce
>>> IOMMUFDDevice
>>>
>>>
>>>
>>> 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
>>>> 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.
>>>>
>>>> Originally-by: Yi Liu <yi.l.liu@intel.com>
>>>> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
>>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>>> ---
>>>>   MAINTAINERS                     |  4 +--
>>>>   include/sysemu/iommufd_device.h | 31 ++++++++++++++++++++
>>>>   backends/iommufd_device.c       | 50
>>> +++++++++++++++++++++++++++++++++
>>> Maybe it is still time to move the iommufd files in a sepate dir, under
>>> hw at the same level as vfio.
>>>
>>> Thoughts?
>>
>> Any reason for the movement? Hw dir contains entries to emulate
>different
>> Devices. Iommufd is not a real device. It's more a backend.
>
>I would include the new services in the existing iommufd .[ch] files.
>No need for a new file since the changes are all related to the IOMMUFD
>device usage.

Make sense, will do.

Thanks
Zhenzhong

Re: [PATCH rfcv1 1/6] backends/iommufd_device: introduce IOMMUFDDevice

Posted by Eric Auger 10 months, 2 weeks ago

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.liu@intel.com>
> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@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.liu@intel.com>
>  M: Eric Auger <eric.auger@redhat.com>
>  M: Zhenzhong Duan <zhenzhong.duan@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.liu@intel.com>
> + *          Zhenzhong Duan <zhenzhong.duan@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.liu@intel.com>
> + *          Zhenzhong Duan <zhenzhong.duan@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

RE: [PATCH rfcv1 1/6] backends/iommufd_device: introduce IOMMUFDDevice

Posted by Duan, Zhenzhong 10 months, 1 week ago


>-----Original Message-----
>From: Eric Auger <eric.auger@redhat.com>
>Subject: Re: [PATCH rfcv1 1/6] backends/iommufd_device: introduce
>IOMMUFDDevice
>
>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

Will do.

>>
>> Originally-by: Yi Liu <yi.l.liu@intel.com>
>> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@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.liu@intel.com>
>>  M: Eric Auger <eric.auger@redhat.com>
>>  M: Zhenzhong Duan <zhenzhong.duan@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.liu@intel.com>
>> + *          Zhenzhong Duan <zhenzhong.duan@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?

' QEMU abstract of Host IOMMUFD device' may be better.

>> + *
>> + * Copyright (C) 2024 Intel Corporation.
>> + *
>> + * Authors: Yi Liu <yi.l.liu@intel.com>
>> + *          Zhenzhong Duan <zhenzhong.duan@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.

Good suggestion, will do.

>> +    } else {
>> +        *type = info.out_data_type;
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>> +void iommufd_device_init(void *_idev, size_t instance_size,
>nit: why the "_"

To distinguish with local idev.

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

VFIO or VDPA may have IOMMUFD related attributes for its own usages.
It looks VFIO doesn't need this for now. I'll remove it, then _idev can be
removed too.

Thanks
Zhenzhong

[PATCH rfcv1 1/6] backends/iommufd_device: introduce IOMMUFDDevice
[PATCH rfcv1 2/6] hw/pci: introduce pci_device_set/unset_iommu_device()
[PATCH rfcv1 3/6] intel_iommu: add set/unset_iommu_device callback
[PATCH rfcv1 4/6] vfio: initialize IOMMUFDDevice and pass to vIOMMU
[PATCH rfcv1 5/6] intel_iommu: extract out vtd_cap_init to initialize cap/ecap
[PATCH rfcv1 6/6] intel_iommu: add a framework to check and sync host IOMMU cap/ecap