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

[Suravee Suthikulanit <suravee.suthikulpanit@amd.com>]

On 5/26/2016 10:44 AM, Jan Beulich wrote:
>>>> Suravee Suthikulanit <suravee.suthikulpanit@amd.com> 05/25/16 9:01 PM >>>
>> On 5/23/2016 6:54 AM, Jan Beulich wrote:
>>>>>> On 22.05.16 at 01:42, <suravee.suthikulpanit@amd.com> wrote:
>>>> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>>>>
>>>> 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().
>
> I think the two things are only lightly related. Changing the order of calls is
> generally fine, but recognizing that guest_iommu_init() really would better be
> called elsewhere makes that re-ordering simply unnecessary.
>
> Jan

So, let discussing these two things separately. I would propose to:

1. Let's just remove the guest_iommu_init() for now since it's not 
functioning, and it seems to not being called at a proper place 
according to Jan. We will revisit this when we re-introduce and fully 
test out the feature.

2. As for the ordering of the iommu_domain_init() and hvm_domain_init() 
, let's continue to discuss to find proper ordering if it needs changing.

Let me know what you guys thinks.

Thanks,
Suravee

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