RE: [PATCH v2 01/10] backends: Introduce abstract HostIOMMUDevice
>-Original Message-
>From: Philippe Mathieu-Daudé
>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
>> Signed-off-by: Zhenzhong Duan
>> ---
>> 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 00..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
RE: [PATCH v2 01/10] backends: Introduce abstract HostIOMMUDevice
>-Original Message-
>From: Cédric Le Goater
>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
>> Signed-off-by: Zhenzhong Duan
>
>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
>
>> 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 00..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 00..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
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
Signed-off-by: Zhenzhong Duan
---
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 00..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
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
Signed-off-by: Zhenzhong Duan
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
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 00..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 00..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
[PATCH v2 01/10] backends: Introduce abstract HostIOMMUDevice
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
Signed-off-by: Zhenzhong Duan
---
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
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 00..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 00..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
--
2.34.1
