diff mbox series

[v2,01/10] backends: Introduce abstract HostIOMMUDevice

Message ID 20240408081230.1030078-2-zhenzhong.duan@intel.com (mailing list archive)
State New, archived
Headers show
Series Add a host IOMMU device abstraction | expand

Commit Message

Duan, Zhenzhong April 8, 2024, 8:12 a.m. UTC 
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>
---
 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

Comments

Cédric Le Goater April 15, 2024, 8:09 a.m. UTC | #1 
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
Philippe Mathieu-Daudé April 15, 2024, 9:18 a.m. UTC | #2 
Hi Zhenzhong,

On 8/4/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>
> ---
>   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/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;
> +
> +    int (*get_host_iommu_info)(HostIOMMUDevice *hiod, void *data, uint32_t len,
> +                               Error **errp);

Please document this new method (in particular return value and @data).

Since @len is sizeof(data), can we use the size_t type?

Thanks,

Phil.

> +};
> +#endif
Duan, Zhenzhong April 15, 2024, 9:57 a.m. UTC | #3 
>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Subject: Re: [PATCH v2 01/10] backends: Introduce abstract
>HostIOMMUDevice
>
>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.

Sure, will doc struct and handlers in v3.

Thanks
Zhenzhong

>
>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
Duan, Zhenzhong April 15, 2024, 9:58 a.m. UTC | #4 
>-----Original Message-----
>From: Philippe Mathieu-Daudé <philmd@linaro.org>
>Subject: Re: [PATCH v2 01/10] backends: Introduce abstract
>HostIOMMUDevice
>
>Hi Zhenzhong,
>
>On 8/4/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>
>> ---
>>   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/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;
>> +
>> +    int (*get_host_iommu_info)(HostIOMMUDevice *hiod, void *data,
>uint32_t len,
>> +                               Error **errp);
>
>Please document this new method (in particular return value and @data).
>
>Since @len is sizeof(data), can we use the size_t type?

Sure, will do.

Thanks
Zhenzhong
diff mbox series

Patch

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