+ Keir (since he added this code originally).
On 05/16/2016 03:19 AM, Paul Durrant wrote:
-----Original Message-----
>From:suravee.suthikulpa...@amd.com
>[mailto:suravee.suthikulpa...@amd.com]
>Sent: 13 May 2016 20:37
>To:xen-devel@lists.xen.org; George Dunlap;jbeul...@suse.com
>Cc: Paul Durrant; Suravee Suthikulpanit; Suravee Suthikulpanit
>Subject: [PATCH V2 2/2] svm: iommu: Only call guest_iommu_init() after
>initialized HVM domain
>
>From: Suravee Suthikulpanit<suravee.suthikulpa...@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 call to guest_iommu_init/destroy() into
>the svm_domain_intialise/_destroy() instead.
>
That seems wrong. You're taking a call that currently comes via a jump table,
i.e. an abstraction layer, and calling it directly. Is it possible, instead, to
move the call to iommu_domain_init() later in arch_domain_create()? It seems
odd, to me at least, that it's done before hvm_domain_initialise() anyway.
Paul
Good point. I think I should be able to move iommu_domain_init() call to
go after hvm_domain_initialise() as the following.
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 9d43f7b..ac289b6 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -625,24 +625,21 @@ int arch_domain_create(struct domain *d, unsigned
int domcr_flags,
if ( (rc = init_domain_irq_mapping(d)) != 0 )
goto fail;
-
- if ( (rc = iommu_domain_init(d)) != 0 )
- goto fail;
}
spin_lock_init(&d->arch.e820_lock);
if ( has_hvm_container_domain(d) )
{
if ( (rc = hvm_domain_initialise(d)) != 0 )
- {
- iommu_domain_destroy(d);
goto fail;
- }
}
else
/* 64-bit PV guest by default. */
d->arch.is_32bit_pv = d->arch.has_32bit_shinfo = 0;
+ if ( !is_idle_domain(d) && (rc = iommu_domain_init(d)) != 0 )
+ goto fail;
+
if ( (rc = psr_domain_init(d)) != 0 )
goto fail;
-----
This was added in the commit 66a882392272346ce1d0bc5a26568894f450a7c0,
and only says initialization cleanup and bugfix. I am not sure what bug
was reported at the time. Anyone has an idea?
Suravee
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
