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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 19.05.16 at 09:52, <Paul.Durrant@xxxxxxxxxx> wrote:
>> >--- 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;
>> 
>> This would in the error case fail to undo what hvm_domain_initialise() did.
>> There was a fix very recently dealing with a similar issue. There really
>> shouldn't be anything that can fail after hvm_domain_initialise().
> 
> Why is that? There are many failure paths within hvm_domain_initialise(). 
> What's wrong with calling hvm_domain_destroy() to undo the whole thing? 

Well, I simply didn't check whether hvm_domain_destroy() would be
suitable to be called in that case, but yes, it looks like it would be.

> (Although I do notice that the io_bitmap would appear to leak in that case... 
> but that looks like a bug to me).

That's a hardware domain only aspect, and failure to construct
the hardware domain would be fatal to the system anyway.

Jan


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