Re: [Xen-devel] [PATCH v3 2/3] svm: iommu: Only call guest_iommu_init() after initialized HVM domain

[Suravee Suthikulanit <suravee.suthikulpanit@xxxxxxx>]

On 5/23/2016 6:54 AM, Jan Beulich wrote:
> > > > On 22.05.16 at 01:42, <suravee.suthikulpanit@xxxxxxx> wrote:
> > From: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
> >
> > The guest_iommu_init() is currently called by the following code path:
> >
> >     arch/x86/domain.c: arch_domain_create()
> >       ]- drivers/passthrough/iommu.c: iommu_domain_init()
> >         |- drivers/passthrough/amd/pci_amd_iommu.c: amd_iommu_domain_init();
> >           |- drivers/passthrough/amd/iommu_guest.c: guest_iommu_init()
> >
> > At this point, the hvm_domain_initialised() has not been called.
> > So register_mmio_handler() in guest_iommu_init() silently fails.
> > This patch moves the iommu_domain_init() to a later point after the
> > hvm_domain_intialise() instead.
> That's one possible approach, which I continue to be not really
> happy with. guest_iommu_init() really is HVM-specific, so maybe
> no longer calling it from amd_iommu_domain_init() would be the
> better solution (instead calling it from hvm_domain_initialise()
> would then seem to be the better option). Thoughts?
Then, this goes back to the approach I proposed in the v1 of this patch 
series, where I call guest_iommu_init/destroy() in the 
svm_domain_initialise/destroy().
However, I'm still not quite clear in why the iommu_domain_init() is 
needed before hvm_domain_initialise().
> In any event is the choice of ...
>
> > @@ -675,6 +675,9 @@ int arch_domain_create(struct domain *d, unsigned int 
> > domcr_flags,
> >
> >      return 0;
> >
> > + fail_1:
> > +    if ( has_hvm_container_domain(d) )
> > +        hvm_domain_destroy(d);
> >   fail:
> >      d->is_dying = DOMDYING_dead;
> >      psr_domain_free(d);
> ... the new label name sub-optimal. Please pick something more
> descriptive, e.g. "iommu_fail", if the current approach is to be
> retained.
>
> Jan
In case we are going with this approach, I will make this change.

Thanks,
Suravee

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel