RE: [PATCH v3 05/19] backends/host_iommu_device: Introduce HostIOMMUDeviceCaps
>-Original Message- >From: Cédric Le Goater >Subject: Re: [PATCH v3 05/19] backends/host_iommu_device: Introduce >HostIOMMUDeviceCaps > >Hello Zhenzhong, > >On 4/29/24 08:50, Zhenzhong Duan wrote: >> HostIOMMUDeviceCaps's elements map to the host IOMMU's capabilities. >> Different platform IOMMU can support different elements. >> >> Currently only two elements, type and aw_bits, type hints the host >> platform IOMMU type, i.e., INTEL vtd, ARM smmu, etc; aw_bits hints >> host IOMMU address width. >> >> Introduce .check_cap() handler to check if >HOST_IOMMU_DEVICE_CAP_XXX >> is supported. >> >> Introduce a HostIOMMUDevice API host_iommu_device_check_cap() >which >> is a wrapper of .check_cap(). >> >> Introduce a HostIOMMUDevice API >host_iommu_device_check_cap_common() >> to check common capabalities of different host platform IOMMUs. >> >> Suggested-by: Cédric Le Goater >> Signed-off-by: Zhenzhong Duan >> --- >> include/sysemu/host_iommu_device.h | 44 >++ >> backends/host_iommu_device.c | 29 >> 2 files changed, 73 insertions(+) >> >> diff --git a/include/sysemu/host_iommu_device.h >b/include/sysemu/host_iommu_device.h >> index 2b58a94d62..12b6afb463 100644 >> --- a/include/sysemu/host_iommu_device.h >> +++ b/include/sysemu/host_iommu_device.h >> @@ -14,12 +14,27 @@ >> >> #include "qom/object.h" >> #include "qapi/error.h" >> +#include "linux/iommufd.h" > > >Please use instead : > >#include Got it. Thanks Zhenzhong
Re: [PATCH v3 05/19] backends/host_iommu_device: Introduce HostIOMMUDeviceCaps
Hello Zhenzhong, On 4/29/24 08:50, Zhenzhong Duan wrote: HostIOMMUDeviceCaps's elements map to the host IOMMU's capabilities. Different platform IOMMU can support different elements. Currently only two elements, type and aw_bits, type hints the host platform IOMMU type, i.e., INTEL vtd, ARM smmu, etc; aw_bits hints host IOMMU address width. Introduce .check_cap() handler to check if HOST_IOMMU_DEVICE_CAP_XXX is supported. Introduce a HostIOMMUDevice API host_iommu_device_check_cap() which is a wrapper of .check_cap(). Introduce a HostIOMMUDevice API host_iommu_device_check_cap_common() to check common capabalities of different host platform IOMMUs. Suggested-by: Cédric Le Goater Signed-off-by: Zhenzhong Duan --- include/sysemu/host_iommu_device.h | 44 ++ backends/host_iommu_device.c | 29 2 files changed, 73 insertions(+) diff --git a/include/sysemu/host_iommu_device.h b/include/sysemu/host_iommu_device.h index 2b58a94d62..12b6afb463 100644 --- a/include/sysemu/host_iommu_device.h +++ b/include/sysemu/host_iommu_device.h @@ -14,12 +14,27 @@ #include "qom/object.h" #include "qapi/error.h" +#include "linux/iommufd.h" Please use instead : #include Thanks, C. + +/** + * struct HostIOMMUDeviceCaps - Define host IOMMU device capabilities. + * + * @type: host platform IOMMU type. + * + * @aw_bits: host IOMMU address width. 0xff if no limitation. + */ +typedef struct HostIOMMUDeviceCaps { +enum iommu_hw_info_type type; +uint8_t aw_bits; +} HostIOMMUDeviceCaps; #define TYPE_HOST_IOMMU_DEVICE "host-iommu-device" OBJECT_DECLARE_TYPE(HostIOMMUDevice, HostIOMMUDeviceClass, HOST_IOMMU_DEVICE) struct HostIOMMUDevice { Object parent_obj; + +HostIOMMUDeviceCaps caps; }; /** @@ -47,5 +62,34 @@ struct HostIOMMUDeviceClass { * Returns: true on success, false on failure. */ bool (*realize)(HostIOMMUDevice *hiod, void *opaque, Error **errp); +/** + * @check_cap: check if a host IOMMU device capability is supported. + * + * Optional callback, if not implemented, hint not supporting query + * of @cap. + * + * @hiod: pointer to a host IOMMU device instance. + * + * @cap: capability to check. + * + * @errp: pass an Error out when fails to query capability. + * + * Returns: <0 on failure, 0 if a @cap is unsupported, or else + * 1 or some positive value for some special @cap, + * i.e., HOST_IOMMU_DEVICE_CAP_AW_BITS. + */ +int (*check_cap)(HostIOMMUDevice *hiod, int cap, Error **errp); }; + +/* + * Host IOMMU device capability list. + */ +#define HOST_IOMMU_DEVICE_CAP_IOMMUFD 0 +#define HOST_IOMMU_DEVICE_CAP_IOMMU_TYPE1 +#define HOST_IOMMU_DEVICE_CAP_AW_BITS 2 + + +int host_iommu_device_check_cap(HostIOMMUDevice *hiod, int cap, Error **errp); +int host_iommu_device_check_cap_common(HostIOMMUDevice *hiod, int cap, + Error **errp); #endif diff --git a/backends/host_iommu_device.c b/backends/host_iommu_device.c index 41f2fdce20..b97d008cc7 100644 --- a/backends/host_iommu_device.c +++ b/backends/host_iommu_device.c @@ -28,3 +28,32 @@ static void host_iommu_device_init(Object *obj) static void host_iommu_device_finalize(Object *obj) { } + +/* Wrapper of HostIOMMUDeviceClass:check_cap */ +int host_iommu_device_check_cap(HostIOMMUDevice *hiod, int cap, Error **errp) +{ +HostIOMMUDeviceClass *hiodc = HOST_IOMMU_DEVICE_GET_CLASS(hiod); +if (!hiodc->check_cap) { +error_setg(errp, ".check_cap() not implemented"); +return -EINVAL; +} + +return hiodc->check_cap(hiod, cap, errp); +} + +/* Implement check on common IOMMU capabilities */ +int host_iommu_device_check_cap_common(HostIOMMUDevice *hiod, int cap, + Error **errp) +{ +HostIOMMUDeviceCaps *caps = >caps; + +switch (cap) { +case HOST_IOMMU_DEVICE_CAP_IOMMU_TYPE: +return caps->type; +case HOST_IOMMU_DEVICE_CAP_AW_BITS: +return caps->aw_bits; +default: +error_setg(errp, "Not support query cap %x", cap); +return -EINVAL; +} +}
Re: [PATCH v3 05/19] backends/host_iommu_device: Introduce HostIOMMUDeviceCaps
On 4/30/24 11:55, Duan, Zhenzhong wrote: -Original Message- From: Cédric Le Goater Subject: Re: [PATCH v3 05/19] backends/host_iommu_device: Introduce HostIOMMUDeviceCaps On 4/29/24 08:50, Zhenzhong Duan wrote: HostIOMMUDeviceCaps's elements map to the host IOMMU's capabilities. Different platform IOMMU can support different elements. Currently only two elements, type and aw_bits, type hints the host platform IOMMU type, i.e., INTEL vtd, ARM smmu, etc; aw_bits hints host IOMMU address width. Introduce .check_cap() handler to check if HOST_IOMMU_DEVICE_CAP_XXX is supported. Introduce a HostIOMMUDevice API host_iommu_device_check_cap() which is a wrapper of .check_cap(). Introduce a HostIOMMUDevice API host_iommu_device_check_cap_common() to check common capabalities of different host platform IOMMUs. Suggested-by: Cédric Le Goater Signed-off-by: Zhenzhong Duan --- include/sysemu/host_iommu_device.h | 44 ++ backends/host_iommu_device.c | 29 2 files changed, 73 insertions(+) diff --git a/include/sysemu/host_iommu_device.h b/include/sysemu/host_iommu_device.h index 2b58a94d62..12b6afb463 100644 --- a/include/sysemu/host_iommu_device.h +++ b/include/sysemu/host_iommu_device.h @@ -14,12 +14,27 @@ #include "qom/object.h" #include "qapi/error.h" +#include "linux/iommufd.h" + +/** + * struct HostIOMMUDeviceCaps - Define host IOMMU device capabilities. + * + * @type: host platform IOMMU type. + * + * @aw_bits: host IOMMU address width. 0xff if no limitation. + */ +typedef struct HostIOMMUDeviceCaps { +enum iommu_hw_info_type type; +uint8_t aw_bits; +} HostIOMMUDeviceCaps; #define TYPE_HOST_IOMMU_DEVICE "host-iommu-device" OBJECT_DECLARE_TYPE(HostIOMMUDevice, HostIOMMUDeviceClass, HOST_IOMMU_DEVICE) struct HostIOMMUDevice { Object parent_obj; + +HostIOMMUDeviceCaps caps; }; /** @@ -47,5 +62,34 @@ struct HostIOMMUDeviceClass { * Returns: true on success, false on failure. */ bool (*realize)(HostIOMMUDevice *hiod, void *opaque, Error **errp); +/** + * @check_cap: check if a host IOMMU device capability is supported. + * + * Optional callback, if not implemented, hint not supporting query + * of @cap. + * + * @hiod: pointer to a host IOMMU device instance. + * + * @cap: capability to check. + * + * @errp: pass an Error out when fails to query capability. + * + * Returns: <0 on failure, 0 if a @cap is unsupported, or else + * 1 or some positive value for some special @cap, + * i.e., HOST_IOMMU_DEVICE_CAP_AW_BITS. + */ +int (*check_cap)(HostIOMMUDevice *hiod, int cap, Error **errp); }; + +/* + * Host IOMMU device capability list. + */ +#define HOST_IOMMU_DEVICE_CAP_IOMMUFD 0 +#define HOST_IOMMU_DEVICE_CAP_IOMMU_TYPE1 +#define HOST_IOMMU_DEVICE_CAP_AW_BITS 2 + + +int host_iommu_device_check_cap(HostIOMMUDevice *hiod, int cap, Error **errp); +int host_iommu_device_check_cap_common(HostIOMMUDevice *hiod, int cap, + Error **errp); #endif diff --git a/backends/host_iommu_device.c b/backends/host_iommu_device.c index 41f2fdce20..b97d008cc7 100644 --- a/backends/host_iommu_device.c +++ b/backends/host_iommu_device.c @@ -28,3 +28,32 @@ static void host_iommu_device_init(Object *obj) static void host_iommu_device_finalize(Object *obj) { } + +/* Wrapper of HostIOMMUDeviceClass:check_cap */ +int host_iommu_device_check_cap(HostIOMMUDevice *hiod, int cap, Error **errp) Since we have an 'Error **errp', we could return a bool instead, unless this is a 'get_cap' routine ? Maybe better to name it host_iommu_device_get_cap()? Because not all results are bool, some are integer, i.e., aw_bits. LGTM. Thanks, C.
RE: [PATCH v3 05/19] backends/host_iommu_device: Introduce HostIOMMUDeviceCaps
>-Original Message- >From: Cédric Le Goater >Subject: Re: [PATCH v3 05/19] backends/host_iommu_device: Introduce >HostIOMMUDeviceCaps > >On 4/29/24 08:50, Zhenzhong Duan wrote: >> HostIOMMUDeviceCaps's elements map to the host IOMMU's capabilities. >> Different platform IOMMU can support different elements. >> >> Currently only two elements, type and aw_bits, type hints the host >> platform IOMMU type, i.e., INTEL vtd, ARM smmu, etc; aw_bits hints >> host IOMMU address width. >> >> Introduce .check_cap() handler to check if >HOST_IOMMU_DEVICE_CAP_XXX >> is supported. >> >> Introduce a HostIOMMUDevice API host_iommu_device_check_cap() >which >> is a wrapper of .check_cap(). >> >> Introduce a HostIOMMUDevice API >host_iommu_device_check_cap_common() >> to check common capabalities of different host platform IOMMUs. >> >> Suggested-by: Cédric Le Goater >> Signed-off-by: Zhenzhong Duan >> --- >> include/sysemu/host_iommu_device.h | 44 >++ >> backends/host_iommu_device.c | 29 >> 2 files changed, 73 insertions(+) >> >> diff --git a/include/sysemu/host_iommu_device.h >b/include/sysemu/host_iommu_device.h >> index 2b58a94d62..12b6afb463 100644 >> --- a/include/sysemu/host_iommu_device.h >> +++ b/include/sysemu/host_iommu_device.h >> @@ -14,12 +14,27 @@ >> >> #include "qom/object.h" >> #include "qapi/error.h" >> +#include "linux/iommufd.h" >> + >> +/** >> + * struct HostIOMMUDeviceCaps - Define host IOMMU device capabilities. >> + * >> + * @type: host platform IOMMU type. >> + * >> + * @aw_bits: host IOMMU address width. 0xff if no limitation. >> + */ >> +typedef struct HostIOMMUDeviceCaps { >> +enum iommu_hw_info_type type; >> +uint8_t aw_bits; >> +} HostIOMMUDeviceCaps; >> >> #define TYPE_HOST_IOMMU_DEVICE "host-iommu-device" >> OBJECT_DECLARE_TYPE(HostIOMMUDevice, HostIOMMUDeviceClass, >HOST_IOMMU_DEVICE) >> >> struct HostIOMMUDevice { >> Object parent_obj; >> + >> +HostIOMMUDeviceCaps caps; >> }; >> >> /** >> @@ -47,5 +62,34 @@ struct HostIOMMUDeviceClass { >>* Returns: true on success, false on failure. >>*/ >> bool (*realize)(HostIOMMUDevice *hiod, void *opaque, Error **errp); >> +/** >> + * @check_cap: check if a host IOMMU device capability is supported. >> + * >> + * Optional callback, if not implemented, hint not supporting query >> + * of @cap. >> + * >> + * @hiod: pointer to a host IOMMU device instance. >> + * >> + * @cap: capability to check. >> + * >> + * @errp: pass an Error out when fails to query capability. >> + * >> + * Returns: <0 on failure, 0 if a @cap is unsupported, or else >> + * 1 or some positive value for some special @cap, >> + * i.e., HOST_IOMMU_DEVICE_CAP_AW_BITS. >> + */ >> +int (*check_cap)(HostIOMMUDevice *hiod, int cap, Error **errp); >> }; >> + >> +/* >> + * Host IOMMU device capability list. >> + */ >> +#define HOST_IOMMU_DEVICE_CAP_IOMMUFD 0 >> +#define HOST_IOMMU_DEVICE_CAP_IOMMU_TYPE1 >> +#define HOST_IOMMU_DEVICE_CAP_AW_BITS 2 >> + >> + >> +int host_iommu_device_check_cap(HostIOMMUDevice *hiod, int cap, >Error **errp); >> +int host_iommu_device_check_cap_common(HostIOMMUDevice *hiod, >int cap, >> + Error **errp); >> #endif >> diff --git a/backends/host_iommu_device.c >b/backends/host_iommu_device.c >> index 41f2fdce20..b97d008cc7 100644 >> --- a/backends/host_iommu_device.c >> +++ b/backends/host_iommu_device.c >> @@ -28,3 +28,32 @@ static void host_iommu_device_init(Object *obj) >> static void host_iommu_device_finalize(Object *obj) >> { >> } >> + >> +/* Wrapper of HostIOMMUDeviceClass:check_cap */ >> +int host_iommu_device_check_cap(HostIOMMUDevice *hiod, int cap, >Error **errp) > >Since we have an 'Error **errp', we could return a bool instead, >unless this is a 'get_cap' routine ? Maybe better to name it host_iommu_device_get_cap()? Because not all results are bool, some are integer, i.e., aw_bits. Thanks Zhenzhong > >Thanks, > >C. > > >> +{ >> +HostIOMMUDeviceClass *hiodc = >HOST_IOMMU_DEVICE_GET_CLASS(hiod); >> +if (!hiodc->check_cap) { >> +error_setg(errp, ".check_cap() not implemented"); >> +return -EINVAL; >> +} >> + >> +return hiodc->check_cap(hiod, cap, errp); >> +} >> + >> +/* Implement check on common IOMMU capabilities */ >> +int host_iommu_device_check_cap_common(HostIOMMUDevice *hiod, >int cap, >> + Error **errp) >> +{ >> +HostIOMMUDeviceCaps *caps = >caps; >> + >> +switch (cap) { >> +case HOST_IOMMU_DEVICE_CAP_IOMMU_TYPE: >> +return caps->type; >> +case HOST_IOMMU_DEVICE_CAP_AW_BITS: >> +return caps->aw_bits; >> +default: >> +error_setg(errp, "Not support query cap %x", cap); >> +return -EINVAL; >> +} >> +}
Re: [PATCH v3 05/19] backends/host_iommu_device: Introduce HostIOMMUDeviceCaps
On 4/29/24 08:50, Zhenzhong Duan wrote: HostIOMMUDeviceCaps's elements map to the host IOMMU's capabilities. Different platform IOMMU can support different elements. Currently only two elements, type and aw_bits, type hints the host platform IOMMU type, i.e., INTEL vtd, ARM smmu, etc; aw_bits hints host IOMMU address width. Introduce .check_cap() handler to check if HOST_IOMMU_DEVICE_CAP_XXX is supported. Introduce a HostIOMMUDevice API host_iommu_device_check_cap() which is a wrapper of .check_cap(). Introduce a HostIOMMUDevice API host_iommu_device_check_cap_common() to check common capabalities of different host platform IOMMUs. Suggested-by: Cédric Le Goater Signed-off-by: Zhenzhong Duan --- include/sysemu/host_iommu_device.h | 44 ++ backends/host_iommu_device.c | 29 2 files changed, 73 insertions(+) diff --git a/include/sysemu/host_iommu_device.h b/include/sysemu/host_iommu_device.h index 2b58a94d62..12b6afb463 100644 --- a/include/sysemu/host_iommu_device.h +++ b/include/sysemu/host_iommu_device.h @@ -14,12 +14,27 @@ #include "qom/object.h" #include "qapi/error.h" +#include "linux/iommufd.h" + +/** + * struct HostIOMMUDeviceCaps - Define host IOMMU device capabilities. + * + * @type: host platform IOMMU type. + * + * @aw_bits: host IOMMU address width. 0xff if no limitation. + */ +typedef struct HostIOMMUDeviceCaps { +enum iommu_hw_info_type type; +uint8_t aw_bits; +} HostIOMMUDeviceCaps; #define TYPE_HOST_IOMMU_DEVICE "host-iommu-device" OBJECT_DECLARE_TYPE(HostIOMMUDevice, HostIOMMUDeviceClass, HOST_IOMMU_DEVICE) struct HostIOMMUDevice { Object parent_obj; + +HostIOMMUDeviceCaps caps; }; /** @@ -47,5 +62,34 @@ struct HostIOMMUDeviceClass { * Returns: true on success, false on failure. */ bool (*realize)(HostIOMMUDevice *hiod, void *opaque, Error **errp); +/** + * @check_cap: check if a host IOMMU device capability is supported. + * + * Optional callback, if not implemented, hint not supporting query + * of @cap. + * + * @hiod: pointer to a host IOMMU device instance. + * + * @cap: capability to check. + * + * @errp: pass an Error out when fails to query capability. + * + * Returns: <0 on failure, 0 if a @cap is unsupported, or else + * 1 or some positive value for some special @cap, + * i.e., HOST_IOMMU_DEVICE_CAP_AW_BITS. + */ +int (*check_cap)(HostIOMMUDevice *hiod, int cap, Error **errp); }; + +/* + * Host IOMMU device capability list. + */ +#define HOST_IOMMU_DEVICE_CAP_IOMMUFD 0 +#define HOST_IOMMU_DEVICE_CAP_IOMMU_TYPE1 +#define HOST_IOMMU_DEVICE_CAP_AW_BITS 2 + + +int host_iommu_device_check_cap(HostIOMMUDevice *hiod, int cap, Error **errp); +int host_iommu_device_check_cap_common(HostIOMMUDevice *hiod, int cap, + Error **errp); #endif diff --git a/backends/host_iommu_device.c b/backends/host_iommu_device.c index 41f2fdce20..b97d008cc7 100644 --- a/backends/host_iommu_device.c +++ b/backends/host_iommu_device.c @@ -28,3 +28,32 @@ static void host_iommu_device_init(Object *obj) static void host_iommu_device_finalize(Object *obj) { } + +/* Wrapper of HostIOMMUDeviceClass:check_cap */ +int host_iommu_device_check_cap(HostIOMMUDevice *hiod, int cap, Error **errp) Since we have an 'Error **errp', we could return a bool instead, unless this is a 'get_cap' routine ? Thanks, C. +{ +HostIOMMUDeviceClass *hiodc = HOST_IOMMU_DEVICE_GET_CLASS(hiod); +if (!hiodc->check_cap) { +error_setg(errp, ".check_cap() not implemented"); +return -EINVAL; +} + +return hiodc->check_cap(hiod, cap, errp); +} + +/* Implement check on common IOMMU capabilities */ +int host_iommu_device_check_cap_common(HostIOMMUDevice *hiod, int cap, + Error **errp) +{ +HostIOMMUDeviceCaps *caps = >caps; + +switch (cap) { +case HOST_IOMMU_DEVICE_CAP_IOMMU_TYPE: +return caps->type; +case HOST_IOMMU_DEVICE_CAP_AW_BITS: +return caps->aw_bits; +default: +error_setg(errp, "Not support query cap %x", cap); +return -EINVAL; +} +}