RE: [PATCH v2 02/10] vfio: Introduce HIODLegacyVFIO device
>-Original Message- >From: Cédric Le Goater >Subject: Re: [PATCH v2 02/10] vfio: Introduce HIODLegacyVFIO device > >Hello, > >On 4/16/24 05:41, Duan, Zhenzhong wrote: >> Hi Cédric, >> >>> -Original Message- >>> From: Cédric Le Goater >>> Subject: Re: [PATCH v2 02/10] vfio: Introduce HIODLegacyVFIO device >>> >>> On 4/8/24 10:12, Zhenzhong Duan wrote: >>>> HIODLegacyVFIO represents a host IOMMU device under VFIO legacy >>>> container backend. >>>> >>>> It includes a link to VFIODevice. >>>> >>>> Suggested-by: Eric Auger >>>> Suggested-by: Cédric Le Goater >>>> Signed-off-by: Zhenzhong Duan >>>> --- >>>>include/hw/vfio/vfio-common.h | 11 +++ >>>>hw/vfio/container.c | 11 ++- >>>>2 files changed, 21 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio- >>> common.h >>>> index b9da6c08ef..f30772f534 100644 >>>> --- a/include/hw/vfio/vfio-common.h >>>> +++ b/include/hw/vfio/vfio-common.h >>>> @@ -31,6 +31,7 @@ >>>>#endif >>>>#include "sysemu/sysemu.h" >>>>#include "hw/vfio/vfio-container-base.h" >>>> +#include "sysemu/host_iommu_device.h" >>>> >>>>#define VFIO_MSG_PREFIX "vfio %s: " >>>> >>>> @@ -147,6 +148,16 @@ typedef struct VFIOGroup { >>>>bool ram_block_discard_allowed; >>>>} VFIOGroup; >>>> >>>> +#define TYPE_HIOD_LEGACY_VFIO TYPE_HOST_IOMMU_DEVICE "- >legacy- >>> vfio" >>> >>> I would prefer to keep the prefix TYPE_HOST_IOMMU_DEVICE. >> >> Will do. >> >>> >>>> +OBJECT_DECLARE_SIMPLE_TYPE(HIODLegacyVFIO, HIOD_LEGACY_VFIO) >>>> + >>>> +/* Abstraction of VFIO legacy host IOMMU device */ >>>> +struct HIODLegacyVFIO { >>> >>> same here >> >> Should I do the same for all the HostIOMMUDevice and >HostIOMMUDeviceClass sub-structures? > >I would for type names. The main reason is for naming consistency, which is >useful for grep and code analysis. Got it. > >> >> The reason I used 'HIOD' abbreviation is some function names become >extremely long >> and exceed 80 characters. E.g.: >> >> @@ -1148,9 +1148,9 @@ static void >vfio_iommu_legacy_class_init(ObjectClass *klass, void *data) >> vioc->pci_hot_reset = vfio_legacy_pci_hot_reset; >> }; >> >> -static int hiod_legacy_vfio_get_host_iommu_info(HostIOMMUDevice >*hiod, >> -void *data, uint32_t len, >> -Error **errp) >> +static int >host_iommu_device_legacy_vfio_get_host_iommu_info(HostIOMMUDevice >*hiod, >> + void *data, >> uint32_t len, >> + Error **errp) >> { >> VFIODevice *vbasedev = HIOD_LEGACY_VFIO(hiod)->vdev; >> /* iova_ranges is a sorted list */ >> @@ -1173,7 +1173,7 @@ static void >hiod_legacy_vfio_class_init(ObjectClass *oc, void *data) >> { >> HostIOMMUDeviceClass *hioc = HOST_IOMMU_DEVICE_CLASS(oc); >> >> -hioc->get_host_iommu_info = hiod_legacy_vfio_get_host_iommu_info; >> +hioc->get_host_iommu_info = >host_iommu_device_legacy_vfio_get_host_iommu_info; >> }; >> >> I didn't find other way to make it meet the 80 chars limitation. Any >suggestions on this? > >Try : > >@@ -1177,7 +1177,8 @@ static void hiod_legacy_vfio_class_init( > { > HostIOMMUDeviceClass *hioc = HOST_IOMMU_DEVICE_CLASS(oc); > >-hioc->get_host_iommu_info = hiod_legacy_vfio_get_host_iommu_info; >+hioc->get_host_iommu_info = >+host_iommu_device_legacy_vfio_get_host_iommu_info; > }; > > static const TypeInfo types[] = { > >That said, I agree that 'host_iommu_device_legacy_vfio' routine prefix >could be shortened to 'hiod_legacy_vfio'. Got it. Thanks Zhenzhong
Re: [PATCH v2 02/10] vfio: Introduce HIODLegacyVFIO device
Hello, On 4/16/24 05:41, Duan, Zhenzhong wrote: Hi Cédric, -Original Message- From: Cédric Le Goater Subject: Re: [PATCH v2 02/10] vfio: Introduce HIODLegacyVFIO device On 4/8/24 10:12, Zhenzhong Duan wrote: HIODLegacyVFIO represents a host IOMMU device under VFIO legacy container backend. It includes a link to VFIODevice. Suggested-by: Eric Auger Suggested-by: Cédric Le Goater Signed-off-by: Zhenzhong Duan --- include/hw/vfio/vfio-common.h | 11 +++ hw/vfio/container.c | 11 ++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio- common.h index b9da6c08ef..f30772f534 100644 --- a/include/hw/vfio/vfio-common.h +++ b/include/hw/vfio/vfio-common.h @@ -31,6 +31,7 @@ #endif #include "sysemu/sysemu.h" #include "hw/vfio/vfio-container-base.h" +#include "sysemu/host_iommu_device.h" #define VFIO_MSG_PREFIX "vfio %s: " @@ -147,6 +148,16 @@ typedef struct VFIOGroup { bool ram_block_discard_allowed; } VFIOGroup; +#define TYPE_HIOD_LEGACY_VFIO TYPE_HOST_IOMMU_DEVICE "-legacy- vfio" I would prefer to keep the prefix TYPE_HOST_IOMMU_DEVICE. Will do. +OBJECT_DECLARE_SIMPLE_TYPE(HIODLegacyVFIO, HIOD_LEGACY_VFIO) + +/* Abstraction of VFIO legacy host IOMMU device */ +struct HIODLegacyVFIO { same here Should I do the same for all the HostIOMMUDevice and HostIOMMUDeviceClass sub-structures? I would for type names. The main reason is for naming consistency, which is useful for grep and code analysis. The reason I used 'HIOD' abbreviation is some function names become extremely long and exceed 80 characters. E.g.: @@ -1148,9 +1148,9 @@ static void vfio_iommu_legacy_class_init(ObjectClass *klass, void *data) vioc->pci_hot_reset = vfio_legacy_pci_hot_reset; }; -static int hiod_legacy_vfio_get_host_iommu_info(HostIOMMUDevice *hiod, -void *data, uint32_t len, -Error **errp) +static int host_iommu_device_legacy_vfio_get_host_iommu_info(HostIOMMUDevice *hiod, + void *data, uint32_t len, + Error **errp) { VFIODevice *vbasedev = HIOD_LEGACY_VFIO(hiod)->vdev; /* iova_ranges is a sorted list */ @@ -1173,7 +1173,7 @@ static void hiod_legacy_vfio_class_init(ObjectClass *oc, void *data) { HostIOMMUDeviceClass *hioc = HOST_IOMMU_DEVICE_CLASS(oc); -hioc->get_host_iommu_info = hiod_legacy_vfio_get_host_iommu_info; +hioc->get_host_iommu_info = host_iommu_device_legacy_vfio_get_host_iommu_info; }; I didn't find other way to make it meet the 80 chars limitation. Any suggestions on this? Try : @@ -1177,7 +1177,8 @@ static void hiod_legacy_vfio_class_init( { HostIOMMUDeviceClass *hioc = HOST_IOMMU_DEVICE_CLASS(oc); -hioc->get_host_iommu_info = hiod_legacy_vfio_get_host_iommu_info; +hioc->get_host_iommu_info = +host_iommu_device_legacy_vfio_get_host_iommu_info; }; static const TypeInfo types[] = { That said, I agree that 'host_iommu_device_legacy_vfio' routine prefix could be shortened to 'hiod_legacy_vfio'. Thanks, C. +/*< private >*/ +HostIOMMUDevice parent; +VFIODevice *vdev; It seems to me that the back pointer should be on the container instead. Looks more correct conceptually. Yes, that makes sense for legacy VFIO, as iova_ranges, pgsizes etc are all saved in bcontainer. +}; + typedef struct VFIODMABuf { QemuDmaBuf buf; uint32_t pos_x, pos_y, pos_updates; diff --git a/hw/vfio/container.c b/hw/vfio/container.c index 77bdec276e..44018ef085 100644 --- a/hw/vfio/container.c +++ b/hw/vfio/container.c @@ -1143,12 +1143,21 @@ static void vfio_iommu_legacy_class_init(ObjectClass *klass, void *data) vioc->pci_hot_reset = vfio_legacy_pci_hot_reset; }; +static void hiod_legacy_vfio_class_init(ObjectClass *oc, void *data) +{ +}; Is it preferable to introduce routines when they are actually useful. Please drop the .class_init definition. Sure. Thanks Zhenzhong Thanks, C. + static const TypeInfo types[] = { { .name = TYPE_VFIO_IOMMU_LEGACY, .parent = TYPE_VFIO_IOMMU, .class_init = vfio_iommu_legacy_class_init, -}, +}, { +.name = TYPE_HIOD_LEGACY_VFIO, +.parent = TYPE_HOST_IOMMU_DEVICE, +.instance_size = sizeof(HIODLegacyVFIO), +.class_init = hiod_legacy_vfio_class_init, +} }; DEFINE_TYPES(types)
RE: [PATCH v2 02/10] vfio: Introduce HIODLegacyVFIO device
Hi Cédric, >-Original Message- >From: Cédric Le Goater >Subject: Re: [PATCH v2 02/10] vfio: Introduce HIODLegacyVFIO device > >On 4/8/24 10:12, Zhenzhong Duan wrote: >> HIODLegacyVFIO represents a host IOMMU device under VFIO legacy >> container backend. >> >> It includes a link to VFIODevice. >> >> Suggested-by: Eric Auger >> Suggested-by: Cédric Le Goater >> Signed-off-by: Zhenzhong Duan >> --- >> include/hw/vfio/vfio-common.h | 11 +++ >> hw/vfio/container.c | 11 ++- >> 2 files changed, 21 insertions(+), 1 deletion(-) >> >> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio- >common.h >> index b9da6c08ef..f30772f534 100644 >> --- a/include/hw/vfio/vfio-common.h >> +++ b/include/hw/vfio/vfio-common.h >> @@ -31,6 +31,7 @@ >> #endif >> #include "sysemu/sysemu.h" >> #include "hw/vfio/vfio-container-base.h" >> +#include "sysemu/host_iommu_device.h" >> >> #define VFIO_MSG_PREFIX "vfio %s: " >> >> @@ -147,6 +148,16 @@ typedef struct VFIOGroup { >> bool ram_block_discard_allowed; >> } VFIOGroup; >> >> +#define TYPE_HIOD_LEGACY_VFIO TYPE_HOST_IOMMU_DEVICE "-legacy- >vfio" > >I would prefer to keep the prefix TYPE_HOST_IOMMU_DEVICE. Will do. > >> +OBJECT_DECLARE_SIMPLE_TYPE(HIODLegacyVFIO, HIOD_LEGACY_VFIO) >> + >> +/* Abstraction of VFIO legacy host IOMMU device */ >> +struct HIODLegacyVFIO { > >same here Should I do the same for all the HostIOMMUDevice and HostIOMMUDeviceClass sub-structures? The reason I used 'HIOD' abbreviation is some function names become extremely long and exceed 80 characters. E.g.: @@ -1148,9 +1148,9 @@ static void vfio_iommu_legacy_class_init(ObjectClass *klass, void *data) vioc->pci_hot_reset = vfio_legacy_pci_hot_reset; }; -static int hiod_legacy_vfio_get_host_iommu_info(HostIOMMUDevice *hiod, -void *data, uint32_t len, -Error **errp) +static int host_iommu_device_legacy_vfio_get_host_iommu_info(HostIOMMUDevice *hiod, + void *data, uint32_t len, + Error **errp) { VFIODevice *vbasedev = HIOD_LEGACY_VFIO(hiod)->vdev; /* iova_ranges is a sorted list */ @@ -1173,7 +1173,7 @@ static void hiod_legacy_vfio_class_init(ObjectClass *oc, void *data) { HostIOMMUDeviceClass *hioc = HOST_IOMMU_DEVICE_CLASS(oc); -hioc->get_host_iommu_info = hiod_legacy_vfio_get_host_iommu_info; +hioc->get_host_iommu_info = host_iommu_device_legacy_vfio_get_host_iommu_info; }; I didn't find other way to make it meet the 80 chars limitation. Any suggestions on this? > >> +/*< private >*/ >> +HostIOMMUDevice parent; >> +VFIODevice *vdev; > >It seems to me that the back pointer should be on the container instead. >Looks more correct conceptually. Yes, that makes sense for legacy VFIO, as iova_ranges, pgsizes etc are all saved in bcontainer. > > >> +}; >> + >> typedef struct VFIODMABuf { >> QemuDmaBuf buf; >> uint32_t pos_x, pos_y, pos_updates; >> diff --git a/hw/vfio/container.c b/hw/vfio/container.c >> index 77bdec276e..44018ef085 100644 >> --- a/hw/vfio/container.c >> +++ b/hw/vfio/container.c >> @@ -1143,12 +1143,21 @@ static void >vfio_iommu_legacy_class_init(ObjectClass *klass, void *data) >> vioc->pci_hot_reset = vfio_legacy_pci_hot_reset; >> }; >> >> +static void hiod_legacy_vfio_class_init(ObjectClass *oc, void *data) >> +{ >> +}; > >Is it preferable to introduce routines when they are actually useful. >Please drop the .class_init definition. Sure. Thanks Zhenzhong > >Thanks, > >C. > > >> + >> static const TypeInfo types[] = { >> { >> .name = TYPE_VFIO_IOMMU_LEGACY, >> .parent = TYPE_VFIO_IOMMU, >> .class_init = vfio_iommu_legacy_class_init, >> -}, >> +}, { >> +.name = TYPE_HIOD_LEGACY_VFIO, >> +.parent = TYPE_HOST_IOMMU_DEVICE, >> +.instance_size = sizeof(HIODLegacyVFIO), >> +.class_init = hiod_legacy_vfio_class_init, >> +} >> }; >> >> DEFINE_TYPES(types)
Re: [PATCH v2 02/10] vfio: Introduce HIODLegacyVFIO device
On 4/8/24 10:12, Zhenzhong Duan wrote: HIODLegacyVFIO represents a host IOMMU device under VFIO legacy container backend. It includes a link to VFIODevice. Suggested-by: Eric Auger Suggested-by: Cédric Le Goater Signed-off-by: Zhenzhong Duan --- include/hw/vfio/vfio-common.h | 11 +++ hw/vfio/container.c | 11 ++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h index b9da6c08ef..f30772f534 100644 --- a/include/hw/vfio/vfio-common.h +++ b/include/hw/vfio/vfio-common.h @@ -31,6 +31,7 @@ #endif #include "sysemu/sysemu.h" #include "hw/vfio/vfio-container-base.h" +#include "sysemu/host_iommu_device.h" #define VFIO_MSG_PREFIX "vfio %s: " @@ -147,6 +148,16 @@ typedef struct VFIOGroup { bool ram_block_discard_allowed; } VFIOGroup; +#define TYPE_HIOD_LEGACY_VFIO TYPE_HOST_IOMMU_DEVICE "-legacy-vfio" I would prefer to keep the prefix TYPE_HOST_IOMMU_DEVICE. +OBJECT_DECLARE_SIMPLE_TYPE(HIODLegacyVFIO, HIOD_LEGACY_VFIO) + +/* Abstraction of VFIO legacy host IOMMU device */ +struct HIODLegacyVFIO { same here +/*< private >*/ +HostIOMMUDevice parent; +VFIODevice *vdev; It seems to me that the back pointer should be on the container instead. Looks more correct conceptually. +}; + typedef struct VFIODMABuf { QemuDmaBuf buf; uint32_t pos_x, pos_y, pos_updates; diff --git a/hw/vfio/container.c b/hw/vfio/container.c index 77bdec276e..44018ef085 100644 --- a/hw/vfio/container.c +++ b/hw/vfio/container.c @@ -1143,12 +1143,21 @@ static void vfio_iommu_legacy_class_init(ObjectClass *klass, void *data) vioc->pci_hot_reset = vfio_legacy_pci_hot_reset; }; +static void hiod_legacy_vfio_class_init(ObjectClass *oc, void *data) +{ +}; Is it preferable to introduce routines when they are actually useful. Please drop the .class_init definition. Thanks, C. + static const TypeInfo types[] = { { .name = TYPE_VFIO_IOMMU_LEGACY, .parent = TYPE_VFIO_IOMMU, .class_init = vfio_iommu_legacy_class_init, -}, +}, { +.name = TYPE_HIOD_LEGACY_VFIO, +.parent = TYPE_HOST_IOMMU_DEVICE, +.instance_size = sizeof(HIODLegacyVFIO), +.class_init = hiod_legacy_vfio_class_init, +} }; DEFINE_TYPES(types)
RE: [PATCH v2 02/10] vfio: Introduce HIODLegacyVFIO device
>-Original Message- >From: Philippe Mathieu-Daudé >Subject: Re: [PATCH v2 02/10] vfio: Introduce HIODLegacyVFIO device > >On 15/4/24 12:10, Duan, Zhenzhong wrote: >> Hi Philippe, >> >>> -Original Message- >>> From: Philippe Mathieu-Daudé >>> Sent: Monday, April 15, 2024 5:20 PM >>> To: Duan, Zhenzhong ; qemu- >>> de...@nongnu.org >>> Cc: alex.william...@redhat.com; c...@redhat.com; >eric.au...@redhat.com; >>> pet...@redhat.com; jasow...@redhat.com; m...@redhat.com; >>> j...@nvidia.com; nicol...@nvidia.com; joao.m.mart...@oracle.com; Tian, >>> Kevin ; Liu, Yi L ; Peng, Chao P >>> >>> Subject: Re: [PATCH v2 02/10] vfio: Introduce HIODLegacyVFIO device >>> >>> On 8/4/24 10:12, Zhenzhong Duan wrote: >>>> HIODLegacyVFIO represents a host IOMMU device under VFIO legacy >>>> container backend. >>>> >>>> It includes a link to VFIODevice. >>>> >>>> Suggested-by: Eric Auger >>>> Suggested-by: Cédric Le Goater >>>> Signed-off-by: Zhenzhong Duan >>>> --- >>>>include/hw/vfio/vfio-common.h | 11 +++ >>>>hw/vfio/container.c | 11 ++- >>>>2 files changed, 21 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio- >>> common.h >>>> index b9da6c08ef..f30772f534 100644 >>>> --- a/include/hw/vfio/vfio-common.h >>>> +++ b/include/hw/vfio/vfio-common.h >>>> @@ -31,6 +31,7 @@ >>>>#endif >>>>#include "sysemu/sysemu.h" >>>>#include "hw/vfio/vfio-container-base.h" >>>> +#include "sysemu/host_iommu_device.h" >>>> >>>>#define VFIO_MSG_PREFIX "vfio %s: " >>>> >>>> @@ -147,6 +148,16 @@ typedef struct VFIOGroup { >>>>bool ram_block_discard_allowed; >>>>} VFIOGroup; >>>> >>>> +#define TYPE_HIOD_LEGACY_VFIO TYPE_HOST_IOMMU_DEVICE "- >legacy- >>> vfio" >>>> +OBJECT_DECLARE_SIMPLE_TYPE(HIODLegacyVFIO, HIOD_LEGACY_VFIO) >>>> + >>>> +/* Abstraction of VFIO legacy host IOMMU device */ >>>> +struct HIODLegacyVFIO { >>>> +/*< private >*/ >>> >>> Please drop this comment. >> >> Will do. But may I ask the rules when to use that comment and when not? > >Sure, see >https://www.qemu.org/docs/master/devel/style.html#qemu-object-model- >declarations Learned, thanks Philippe. BRs. Zhenzhong
Re: [PATCH v2 02/10] vfio: Introduce HIODLegacyVFIO device
On 15/4/24 12:10, Duan, Zhenzhong wrote: Hi Philippe, -Original Message- From: Philippe Mathieu-Daudé Sent: Monday, April 15, 2024 5:20 PM To: Duan, Zhenzhong ; qemu- de...@nongnu.org Cc: alex.william...@redhat.com; c...@redhat.com; eric.au...@redhat.com; pet...@redhat.com; jasow...@redhat.com; m...@redhat.com; j...@nvidia.com; nicol...@nvidia.com; joao.m.mart...@oracle.com; Tian, Kevin ; Liu, Yi L ; Peng, Chao P Subject: Re: [PATCH v2 02/10] vfio: Introduce HIODLegacyVFIO device On 8/4/24 10:12, Zhenzhong Duan wrote: HIODLegacyVFIO represents a host IOMMU device under VFIO legacy container backend. It includes a link to VFIODevice. Suggested-by: Eric Auger Suggested-by: Cédric Le Goater Signed-off-by: Zhenzhong Duan --- include/hw/vfio/vfio-common.h | 11 +++ hw/vfio/container.c | 11 ++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio- common.h index b9da6c08ef..f30772f534 100644 --- a/include/hw/vfio/vfio-common.h +++ b/include/hw/vfio/vfio-common.h @@ -31,6 +31,7 @@ #endif #include "sysemu/sysemu.h" #include "hw/vfio/vfio-container-base.h" +#include "sysemu/host_iommu_device.h" #define VFIO_MSG_PREFIX "vfio %s: " @@ -147,6 +148,16 @@ typedef struct VFIOGroup { bool ram_block_discard_allowed; } VFIOGroup; +#define TYPE_HIOD_LEGACY_VFIO TYPE_HOST_IOMMU_DEVICE "-legacy- vfio" +OBJECT_DECLARE_SIMPLE_TYPE(HIODLegacyVFIO, HIOD_LEGACY_VFIO) + +/* Abstraction of VFIO legacy host IOMMU device */ +struct HIODLegacyVFIO { +/*< private >*/ Please drop this comment. Will do. But may I ask the rules when to use that comment and when not? Sure, see https://www.qemu.org/docs/master/devel/style.html#qemu-object-model-declarations I see some QOM use that comment to mark private vs. public, for example: struct AccelState { /*< private >*/ Object parent_obj; This is old style which might be cleaned some day... }; typedef struct AccelClass { /*< private >*/ ObjectClass parent_class; /*< public >*/ +HostIOMMUDevice parent; Please name 'parent_obj'. Will do. Thanks, Phil. Thanks Zhenzhong +VFIODevice *vdev; +};
RE: [PATCH v2 02/10] vfio: Introduce HIODLegacyVFIO device
Hi Philippe, >-Original Message- >From: Philippe Mathieu-Daudé >Sent: Monday, April 15, 2024 5:20 PM >To: Duan, Zhenzhong ; qemu- >de...@nongnu.org >Cc: alex.william...@redhat.com; c...@redhat.com; eric.au...@redhat.com; >pet...@redhat.com; jasow...@redhat.com; m...@redhat.com; >j...@nvidia.com; nicol...@nvidia.com; joao.m.mart...@oracle.com; Tian, >Kevin ; Liu, Yi L ; Peng, Chao P > >Subject: Re: [PATCH v2 02/10] vfio: Introduce HIODLegacyVFIO device > >On 8/4/24 10:12, Zhenzhong Duan wrote: >> HIODLegacyVFIO represents a host IOMMU device under VFIO legacy >> container backend. >> >> It includes a link to VFIODevice. >> >> Suggested-by: Eric Auger >> Suggested-by: Cédric Le Goater >> Signed-off-by: Zhenzhong Duan >> --- >> include/hw/vfio/vfio-common.h | 11 +++ >> hw/vfio/container.c | 11 ++- >> 2 files changed, 21 insertions(+), 1 deletion(-) >> >> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio- >common.h >> index b9da6c08ef..f30772f534 100644 >> --- a/include/hw/vfio/vfio-common.h >> +++ b/include/hw/vfio/vfio-common.h >> @@ -31,6 +31,7 @@ >> #endif >> #include "sysemu/sysemu.h" >> #include "hw/vfio/vfio-container-base.h" >> +#include "sysemu/host_iommu_device.h" >> >> #define VFIO_MSG_PREFIX "vfio %s: " >> >> @@ -147,6 +148,16 @@ typedef struct VFIOGroup { >> bool ram_block_discard_allowed; >> } VFIOGroup; >> >> +#define TYPE_HIOD_LEGACY_VFIO TYPE_HOST_IOMMU_DEVICE "-legacy- >vfio" >> +OBJECT_DECLARE_SIMPLE_TYPE(HIODLegacyVFIO, HIOD_LEGACY_VFIO) >> + >> +/* Abstraction of VFIO legacy host IOMMU device */ >> +struct HIODLegacyVFIO { >> +/*< private >*/ > >Please drop this comment. Will do. But may I ask the rules when to use that comment and when not? I see some QOM use that comment to mark private vs. public, for example: struct AccelState { /*< private >*/ Object parent_obj; }; typedef struct AccelClass { /*< private >*/ ObjectClass parent_class; /*< public >*/ > >> +HostIOMMUDevice parent; > >Please name 'parent_obj'. Will do. Thanks Zhenzhong > >> +VFIODevice *vdev; >> +};
Re: [PATCH v2 02/10] vfio: Introduce HIODLegacyVFIO device
On 8/4/24 10:12, Zhenzhong Duan wrote: HIODLegacyVFIO represents a host IOMMU device under VFIO legacy container backend. It includes a link to VFIODevice. Suggested-by: Eric Auger Suggested-by: Cédric Le Goater Signed-off-by: Zhenzhong Duan --- include/hw/vfio/vfio-common.h | 11 +++ hw/vfio/container.c | 11 ++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h index b9da6c08ef..f30772f534 100644 --- a/include/hw/vfio/vfio-common.h +++ b/include/hw/vfio/vfio-common.h @@ -31,6 +31,7 @@ #endif #include "sysemu/sysemu.h" #include "hw/vfio/vfio-container-base.h" +#include "sysemu/host_iommu_device.h" #define VFIO_MSG_PREFIX "vfio %s: " @@ -147,6 +148,16 @@ typedef struct VFIOGroup { bool ram_block_discard_allowed; } VFIOGroup; +#define TYPE_HIOD_LEGACY_VFIO TYPE_HOST_IOMMU_DEVICE "-legacy-vfio" +OBJECT_DECLARE_SIMPLE_TYPE(HIODLegacyVFIO, HIOD_LEGACY_VFIO) + +/* Abstraction of VFIO legacy host IOMMU device */ +struct HIODLegacyVFIO { +/*< private >*/ Please drop this comment. +HostIOMMUDevice parent; Please name 'parent_obj'. +VFIODevice *vdev; +};