RE: [PATCH v1 04/22] hw/iommu: introduce HostIOMMUContext

Liu, Yi L
2020-03-24T10:01:07Z

> From: Peter Xu <peterx@redhat.com>
> Sent: Tuesday, March 24, 2020 4:59 AM
> To: Liu, Yi L <yi.l.liu@intel.com>
> Subject: Re: [PATCH v1 04/22] hw/iommu: introduce HostIOMMUContext
>
> On Sun, Mar 22, 2020 at 05:36:01AM -0700, Liu Yi L wrote:
[...]
> > +
> > +void host_iommu_ctx_init(void *_host_icx, size_t instance_size,
> > + const char *mrtypename,
> > + uint64_t flags)
> > +{
> > + HostIOMMUContext *host_icx;
> > +
> > + object_initialize(_host_icx, instance_size, mrtypename);
> > + host_icx = HOST_IOMMU_CONTEXT(_host_icx);
> > + host_icx->flags = flags;
> > + host_icx->initialized = true;
> > +}
> > +
> > +void host_iommu_ctx_destroy(HostIOMMUContext *host_icx)
> > +{
> > + host_icx->flags = 0x0;
> > + host_icx->initialized = false;
> > +}
>
> Can we simply put this into .instance_finalize() and be called
> automatically when the object loses the last refcount?
>
> Actually an easier way may be dropping this directly.. If the object
> is to be destroyed then IMHO we don't need to care about flags at all,
> we just free memories we use, but for this object it's none.

right, I'll drop this function. :-)

> > +
> > +static void host_icx_init_fn(Object *obj)
> > +{
> > + HostIOMMUContext *host_icx = HOST_IOMMU_CONTEXT(obj);
> > +
> > + host_icx->flags = 0x0;
> > + host_icx->initialized = false;
>
> Here is also a bit strange... IIUC the only way to init this object
> is via host_iommu_ctx_init() where all these flags will be set. But
> if so, then we're setting all these twice always. Maybe this function
> can be dropped too?

yes, it is. At least, it is not necessary for now. Will drop it.

Thanks,
Yi Liu