Hi Yi, On 4/6/20 10:04 AM, Liu, Yi L wrote: > Hi Eric, > >> From: Auger Eric < eric.auger@xxxxxxxxxx> >> Sent: Tuesday, March 31, 2020 1:23 AM >> To: Liu, Yi L <yi.l.liu@xxxxxxxxx>; qemu-devel@xxxxxxxxxx; >> Subject: Re: [PATCH v2 04/22] hw/iommu: introduce HostIOMMUContext >> >> Yi, >> >> On 3/30/20 6:24 AM, Liu Yi L wrote: >>> Currently, many platform vendors provide the capability of dual stage >>> DMA address translation in hardware. For example, nested translation >>> on Intel VT-d scalable mode, nested stage translation on ARM SMMUv3, >>> and etc. In dual stage DMA address translation, there are two stages >>> address translation, stage-1 (a.k.a first-level) and stage-2 (a.k.a >>> second-level) translation structures. Stage-1 translation results are >>> also subjected to stage-2 translation structures. Take vSVA (Virtual >>> Shared Virtual Addressing) as an example, guest IOMMU driver owns >>> stage-1 translation structures (covers GVA->GPA translation), and host >>> IOMMU driver owns stage-2 translation structures (covers GPA->HPA >>> translation). VMM is responsible to bind stage-1 translation structures >>> to host, thus hardware could achieve GVA->GPA and then GPA->HPA >>> translation. For more background on SVA, refer the below links. >>> - https://www.youtube.com/watch?v=Kq_nfGK5MwQ >>> - https://events19.lfasiallc.com/wp-content/uploads/2017/11/\ >>> Shared-Virtual-Memory-in-KVM_Yi-Liu.pdf >>> > [...] >>> +void host_iommu_ctx_init(void *_iommu_ctx, size_t instance_size, >>> + const char *mrtypename, >>> + uint64_t flags) >>> +{ >>> + HostIOMMUContext *iommu_ctx; >>> + >>> + object_initialize(_iommu_ctx, instance_size, mrtypename); >>> + iommu_ctx = HOST_IOMMU_CONTEXT(_iommu_ctx); >>> + iommu_ctx->flags = flags; >>> + iommu_ctx->initialized = true; >>> +} >>> + >>> +static const TypeInfo host_iommu_context_info = { >>> + .parent = TYPE_OBJECT, >>> + .name = TYPE_HOST_IOMMU_CONTEXT, >>> + .class_size = sizeof(HostIOMMUContextClass), >>> + .instance_size = sizeof(HostIOMMUContext), >>> + .abstract = true, >> Can't we use the usual .instance_init and .instance_finalize? > sorry, I somehow missed this comment. In prior patch, .instace_init > was used, but the current major init method is via host_iommu_ctx_init(), > so .instance_init is not really necessary. > https://www.spinics.net/lists/kvm/msg210878.html OK globally what disturbs me is you introduced a QOM object but globally the inheritance schema is not totally clear to me (only a VFIO derived is created and I do not understand what other backend would be able to use it) and this does not really have the look & feel of standard QOM objects. I tried to compare its usage/implementation version MemoryRegion for instance. Thanks Eric > Regards, > Yi Liu >
